Tests: Contract test for CCoinsView and CCoinsViewBacked #15137

pull mmachicao wants to merge 1 commits into bitcoin:master from mmachicao:coins_contract_tests changing 1 files +72 −0

mmachicao commented at 11:10 PM on January 9, 2019: contributor
Added some admittedly trivial tests to protect against unintended and breaking modifications of the respective contracts.

Remark: AFAIK the only specialization in production code of CCoinsView is CCoinsViewDB On the other hand, there are several parts in the production code where a DUMMY instance of CCoinsView is instantiated only to comply with the signature of the class CCoinsViewCache.

IMHO this smells like overengineering (aka: the future never comes) or forced code recycling and could potentially lead to unintended behavior along the road.

Will invest some time to take a deeper look into this.

e.g.
```
./bitcoin/src/rpc/rawtransaction.cpp:    CCoinsView viewDummy;
./bitcoin/src/rpc/rawtransaction.cpp:    CCoinsViewCache view(&viewDummy);

./bitcoin/src/bitcoin-tx.cpp:    CCoinsView viewDummy;
./bitcoin/src/bitcoin-tx.cpp:    CCoinsViewCache view(&viewDummy);

./bitcoin/src/validation.cpp:        CCoinsView dummy;
./bitcoin/src/validation.cpp:        CCoinsViewCache view(&dummy);

./bitcoin/src/bench/ccoins_caching.cpp:    CCoinsView coinsDummy;
./bitcoin/src/bench/ccoins_caching.cpp:    CCoinsViewCache coins(&coinsDummy);
```
fanquake added the label Tests on Jan 9, 2019

in src/test/coins_tests.cpp:144 in 6e541291d4 outdated

 139 | +
 140 | +    // Provide an alternaitve implementation of CCoinsView with the opposite behavior
 141 | +    // and test that CCoinsViewBackend behaves accordingly
 142 | +    class Opposite : public CCoinsView
 143 | +    {
 144 | +        bool GetCoin(const COutPoint& outpoint, Coin& coin) const override { return true; }

practicalswift commented at 5:47 PM on January 10, 2019:

Choose another name for coin to avoid shadowing coin on L127?

in src/test/coins_tests.cpp:145 in 6e541291d4 outdated

 140 | +    // Provide an alternaitve implementation of CCoinsView with the opposite behavior
 141 | +    // and test that CCoinsViewBackend behaves accordingly
 142 | +    class Opposite : public CCoinsView
 143 | +    {
 144 | +        bool GetCoin(const COutPoint& outpoint, Coin& coin) const override { return true; }
 145 | +        bool BatchWrite(CCoinsMap& mapCoins, const uint256& hashBlock) override { return true; }

practicalswift commented at 5:48 PM on January 10, 2019:

Same here but for hashBlock.

Tests: Contract test for CCoinsView and CCoinsViewBacked

Also:

Use override keyword on overriden methods
Rename variables with potentially conflicting names

24ec978f91

in src/test/coins_tests.cpp:140 in 6e541291d4 outdated

 135 | +    uint256 hashBlock;
 136 | +    BOOST_CHECK_EQUAL(vBacked.BatchWrite(map, hashBlock), false);
 137 | +    BOOST_CHECK(vBacked.Cursor() == nullptr);
 138 | +    BOOST_CHECK_EQUAL(vBacked.EstimateSize(), 0);
 139 | +
 140 | +    // Provide an alternaitve implementation of CCoinsView with the opposite behavior

practicalswift commented at 5:48 PM on January 10, 2019:

Should be "alternative" :-)

mmachicao commented at 3:35 PM on January 11, 2019:

Done. Unfortunately, the build environment broke down.

mmachicao closed this on Aug 3, 2019
DrahtBot locked this on Dec 16, 2021

Contributors

mmachicao

practicalswift

Labels

Tests