Page MenuHomePhabricator

[avalanche] Do not modify the chaintip in stake contender test
ClosedPublic

Authored by roqqit on Tue, Nov 19, 21:08.

Details

Reviewers
Fabien
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Restricted Project
Commits
rABCd74a3bd8a0e8: [avalanche] Do not modify the chaintip in stake contender test
Summary

We could take a lock on cs_main to do the modification here, but it's better to just set the mock time prior to mining the block in the first place.

Test Plan

This does not replicate on my machine but it should fix the CI TSAN failure:
https://build.bitcoinabc.org/buildConfiguration/BitcoinABC_ResourceIntensiveBuilds_BitcoinAbcMasterTsan/853976?buildTab=log&linesState=91&logView=flowAware&focusLine=2282

cmake -GNinja .. -DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++ -DCMAKE_BUILD_TYPE=Debug -DENABLE_SANITIZERS=thread
ninja check-avalanche-processor_tests

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Owners added a reviewer: Restricted Owners Package.Tue, Nov 19, 21:08
roqqit requested review of this revision.Tue, Nov 19, 21:08
Fabien requested changes to this revision.Tue, Nov 19, 21:11
Fabien added a subscriber: Fabien.
Fabien added inline comments.
src/avalanche/test/processor_tests.cpp
2454 ↗(On Diff #50960)

You should take the lock only once here ? Move the call into the below scoped lock

This revision now requires changes to proceed.Tue, Nov 19, 21:11
roqqit retitled this revision from [avalanche] Add missing lock in stake contenders test to [avalanche] Do not modify the chaintip in stake contender test.Tue, Nov 19, 21:23
roqqit edited the summary of this revision. (Show Details)
This revision is now accepted and ready to land.Wed, Nov 20, 08:43