Skip to content

Conversation

@ryanofsky
Copy link
Contributor

@ryanofsky ryanofsky commented Dec 9, 2016

This change adds a check for the validity of the FRESH flag in CCoinsViewCache::BatchWrite. The check added here is analogous to the assert @morcos is adding in #9107. It is checking the same condition, just inside a different method (BatchWrite instead of ModifyNewCoins).

@ryanofsky ryanofsky force-pushed the pr/coins-batch-assert branch 3 times, most recently from 02e6924 to 376d435 Compare December 9, 2016 21:08
@fanquake
Copy link
Member

Travis is failing on Windows

FAIL: test/test_bitcoin
=======================
Running A218 test cases...
ssertion failed!
Program: Z:\home\travis\build\bitcoin\bitcoin\build\src\test\test_bitcoin.exe
File: ../../src/coins.cpp, Line 194
Expression: !(it->second.flags & CCoinsCacheEntry::FRESH) || itUs->second.coins.IsPruned()

@ryanofsky ryanofsky force-pushed the pr/coins-batch-assert branch 2 times, most recently from c3ba2d3 to 3d742a0 Compare December 20, 2016 00:00
@ryanofsky ryanofsky force-pushed the pr/coins-batch-assert branch from 3d742a0 to ac10e17 Compare December 21, 2016 13:41
@ryanofsky ryanofsky changed the title Assert FRESH validity in CCoinsViewCache::BatchWrite (on top of #9308) Assert FRESH validity in CCoinsViewCache::BatchWrite Dec 21, 2016
@ryanofsky
Copy link
Contributor Author

Switched from assert to throw std::logic_error to fix the windows tests because boost execution monitor isn't able to catch asserts on windows. (Previous implementation with boost execution monitor is here: ryanofsky@c3ba2d3)

@sipa
Copy link
Member

sipa commented Jan 3, 2017

utACK ac10e17

@ryanofsky ryanofsky force-pushed the pr/coins-batch-assert branch from ac10e17 to dd44ea3 Compare January 4, 2017 20:15
@ryanofsky
Copy link
Contributor Author

Rebased to avoid merge conflict (in ccoins_test) with #9107.

@morcos
Copy link
Contributor

morcos commented Jan 5, 2017

utACK dd44ea3

@TheBlueMatt
Copy link
Contributor

utACK dd44ea3 (didn't look too hard at tests, though).

@sipa sipa merged commit dd44ea3 into bitcoin:master Jan 9, 2017
sipa added a commit that referenced this pull request Jan 9, 2017
dd44ea3 Check FRESH validity in CCoinsViewCache::BatchWrite (Russell Yanofsky)
codablock pushed a commit to codablock/dash that referenced this pull request Oct 23, 2017
dd44ea3 Check FRESH validity in CCoinsViewCache::BatchWrite (Russell Yanofsky)
random-zebra added a commit to PIVX-Project/PIVX that referenced this pull request Aug 18, 2020
cdbb1a6 Remove unused variable in test, fixing warning. (Russell Yanofsky)
03031a5 Check FRESH validity in CCoinsViewCache::BatchWrite (Russell Yanofsky)
3fb3e03 Fix dangerous condition in ModifyNewCoins. (random-zebra)
228d6bc [test] Add CCoinsViewCache Access/Modify/Write tests (random-zebra)

Pull request description:

  Going on with the coins view cache update work (started with #1774 and #1777).
  This backports the following upstream PRs (adapting them with the assumption that duplicate coinbase transactions are not possible):

  - bitcoin#9308 - [test] Add CCoinsViewCache Access/Modify/Write tests
  - bitcoin#9107 - Safer modify new coins
  - bitcoin#9310 - Assert FRESH validity in CCoinsViewCache::BatchWrite
  - bitcoin#9435 - Removed unused variable in test, fixing warning.

  Note: Will rebase #1773 on top of this one (to change `ApplyTxInUndo` return value in the unit test too).

ACKs for top commit:
  furszy:
    pretty nice one, ACK cdbb1a6
  Fuzzbawls:
    ACK cdbb1a6

Tree-SHA512: 69c534da1083c3c4a535923e98da7750474c6698bb45e042778b539a27fab0455d78aaf670bed9485522f9d2ee25e4ad57fa7d35632523fe7376fe5f5febb1e5
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants