As per title.
Details
Details
- Reviewers
freetrader sickpig kyuupichan awemany - Group Reviewers
Restricted Project - Commits
- rSTAGINGde3a474b1e9d: Some simplification in the compact block code and tests
rABCde3a474b1e9d: Some simplification in the compact block code and tests
make check ../qa/pull-tester/rpc-tests.py
Diff Detail
Diff Detail
- Repository
- rABC Bitcoin ABC
- Branch
- compactsimplify
- Lint
Lint Passed - Unit
No Test Coverage - Build Status
Buildable 351 Build 351: arc lint + arc unit
Event Timeline
| src/net_processing.cpp | ||
|---|---|---|
| 1630 | This appears similarly redundant, also the if (!State(...)) before. Furthermore, is it the right behavior to simply ignore a wrong-version compact block (as that appears to be what is happening here)? | |
| src/net_processing.cpp | ||
|---|---|---|
| 1622 | This actually also appears to be redundant... | |
Comment Actions
See comments, make check passes.
| src/net_processing.cpp | ||
|---|---|---|
| 1622 | As I said in slack, State(pfrom->GetId())->fProvidesHeaderAndIDs is checked for being false and then State(pfrom->GetId())->fProvidesHeaderAndIDs is set to true, which appears redundant. fProvidesHeaderAndIDs is also a simple bool flag and not some complex object with an overloaded assignment operator. When I asked you on this you said 'side effects, multithreading'. But if there is a multithreading side-effect problem here, shouldn't the if(..) part be locked somehow? Anyways, do as you think makes the most sense. | |
| 1630 | Ok, makes sense on the SegWit part. Otherwise, same comment as above. | |
| src/net_processing.cpp | ||
|---|---|---|
| 1622 | In one case there is a write, in the other case there is no write. This is semantic change. | |
Comment Actions
Accepted this on account of argument against semantic change.
If there was such a semantic change it would need separate testing to show it is ok.