This moves the logic out of the compact block into a new ShortIdProcessor that can be reused for the compact proofs. This also makes the code a bit easier to follow.
Depends on D11569.
Details
Details
- Reviewers
sdulfari - Group Reviewers
Restricted Project - Commits
- rABCeb09c72ebc66: Extract the compact block processing into a generic helper class
ninja all check-all
Diff Detail
Diff Detail
- Repository
- rABC Bitcoin ABC
- Branch
- factorize_compact_proofs_blocks
- Lint
Lint Passed - Unit
No Test Coverage - Build Status
Buildable 19258 Build 38263: Build Diff lint-circular-dependencies · build-clang · build-clang-tidy · build-without-wallet · build-debug · build-diff Build 38262: arc lint + arc unit
Event Timeline
| src/shortidprocessor.h | ||
|---|---|---|
| 18–19 | I was guilty of not doing that myself, but you might want to use private inheritance to benefit from empty base optimization. | |
| src/shortidprocessor.h | ||
|---|---|---|
| 18–19 | TIL | |
Comment Actions
The tests aren't comprehensive enough.
| src/test/shortidprocessor_tests.cpp | ||
|---|---|---|
| 53 ↗ | (On Diff #33855) | Using 0 as the bucket size seems wrong for both of the above cases since it's not intuitive, even though it certainly makes writing it easy. It raises questions like: What does a size 0 mean? Is size 0 related to any real world case that is conceivable? I would also consider disallowing size 0 entirely but that should not block this diff. |
| 73 ↗ | (On Diff #33855) | This tests one specific case where the first index is out of bounds, but does not test for invalid indices following valid ones and vice versa. |
| 78 ↗ | (On Diff #33855) | What about shortid out of bounds indices? |
| src/test/shortidprocessor_tests.cpp | ||
|---|---|---|
| 53 ↗ | (On Diff #33855) | 0 is not a realistic value but is the only value that is guaranteed to trigger the flag all the time, since the number of bucket is set to the number of shortids. Other values depends on statistics and would make the test flaky. I'll add a comment for that. |
| 78 ↗ | (On Diff #33855) | I will not create a vector with max(uint32_t) elements to check this case :) |
Comment Actions
Add a comment for the uneven bucket case, expand on the prefilled tx index out of bounds