Backport PR12151 https://github.com/bitcoin/bitcoin/pull/12151/commits
54dc13b rpc: Fix SoftForkMajorityDesc and SoftForkDesc signatures
343b98c rpc: Specify chain tip instead of chain in GetDifficulty
b9f226b rpc: Remove cs_main lock from blockToJSON and blockHeaderToJSON
Backport PR15932 https://github.com/bitcoin/bitcoin/pull/15932/commits
fab00a5 rpc: Serialize in getblock without cs_main
faea564 rpc: Add lock annotations to block{,header}ToJSON
Details
Details
- Reviewers
deadalnix markblundeberg - Group Reviewers
Restricted Project Restricted Owners Package (Owns No Changed Paths)
make check
test_runner.py
Diff Detail
Diff Detail
- Repository
- rABC Bitcoin ABC
- Branch
- rpc_block_nolock
- Lint
Lint Passed - Unit
No Test Coverage - Build Status
Buildable 5965 Build 9990: Bitcoin ABC Buildbot (legacy) Build 9989: arc lint + arc unit
Event Timeline
Comment Actions
Hey, just a heads up that this partly overlaps with D3021. Also when you're backporting things from Bitcoin Core, please make sure to mark them as backports and mention the PR number in the description.
Hopefully D3021 / grandparent D3014 can get merged one day and then let's merge these backports too.
Comment Actions
Please mention the PR that is backport in the description like this:
- This is backport of Core PR12345
- The test plan is inadequate as it doesn't run the RPC tests - and probably should do so using the debug build to check that there are at least no regression (that build is not green for RPC tests).
- This seems to be a duplicate of D3021 and suffer from the same problem that there is a test dependency that is missing - or if it is not, then there should be at least some justification as to why this is the case.
Comment Actions
@IntegralTeam OK, now my diff is landed -- please rebase onto master, and edit the description to say that this is a backport like so:
PR15932 backport https://github.com/bitcoin/bitcoin/pull/15932/files
As Amaury says, please also add test_runner.py to the test plan.
Comment Actions
Now that PR12151 is already backported in a prior diff (D3021) you should take it out of the description, and just focus this on PR15932. :-)
| src/rest.cpp | ||
|---|---|---|
| 197 | leave out this change | |
| src/rpc/blockchain.cpp | ||
| 812 | Original PR brings in RPCHelpMan here and IsValidNumArgs below. If we take this partial backport are we sure to remember to introduce these later on? | |
| 907 | The original PR uses GetBlockChecked() instead of these two if-blocks... it looks like the two ways are equivalent but out-of-order backporting makes it tricky to review. Is it not easily possible to backport the priors first? | |
| src/rpc/blockchain.h | ||
| 12 | stdint.h -> cstdint | |
| src/rpc/blockchain.cpp | ||
|---|---|---|
| 812 | @markblundeberg I've look at this functions, it's include some kind of PR's. As a minimum next PR's:
This changes are simple but affect on a much part of files. Would be better create another PR for it. | |