This patch brings in the ability for pruning nodes to use getrawtransaction by including a blockhash, which has recently been included in bitcoin (https://github.com/bitcoin/bitcoin/pull/10275). Since pruning nodes cannot use the -txindex option, now as long as they know the blockhash which contains the transaction they're looking for (and it hasn't been pruned) then they can retrieve the raw transaction.
Details
Details
Diff Detail
Diff Detail
- Repository
- rABC Bitcoin ABC
- Branch
- getrawtransaction-pruned-node-support
- Lint
Lint Passed - Unit
No Test Coverage - Build Status
Buildable 3085 Build 4257: Bitcoin ABC Buildbot (legacy) Build 4256: arc lint + arc unit
Event Timeline
| src/validation.cpp | ||
|---|---|---|
| 609 ↗ | (On Diff #4583) | Did clang-format change this? Looks unnecessary. |
| src/rpc/rawtransaction.cpp | ||
|---|---|---|
| 241 ↗ | (On Diff #4583) | we're moving towards lowerSnakeCasing for our naming conventions. please use inActiveChain instead. |
| 275 ↗ | (On Diff #4583) | This doesn't look related to the rest of the diff. |
| src/validation.cpp | ||
| 609 ↗ | (On Diff #4583) | Yes, this is definitely from clang-format. Please use clang-format v4.0 when linting. |
| 1118 ↗ | (On Diff #4583) | Add a comment for what blockIndex does. |
| 1122 ↗ | (On Diff #4583) | blockIndex doesn't appear to be used. I think a bool flag would be better suited here. something like fSkipMempool or something like that |
| 1127 ↗ | (On Diff #4583) | Add a comment to explain why this block is skipped when blockIndex is set. |
Comment Actions
Ammendments after first review feedback.
This removes the unnecessary in_active_chain flag, adds some additional comments, and fixes a bug where blockIndex was not actually being utilized.
Comment Actions
This does only port a small portion of the original PR. Notably missing are the tests.
Comment Actions
I'm in the middle of a move at the moment and away from my main PC, but I should be set up again next week and will work on incorporating the test cases then.
| src/rpc/rawtransaction.cpp | ||
|---|---|---|
| 275 ↗ | (On Diff #4583) | I think you're right. It was in the original patch but I don't see how it's necessary for this change. I'll remove it. |
| src/validation.cpp | ||
| 609 ↗ | (On Diff #4583) | Yes, yes it did. It changed formatting in a couple other areas as well (line 1121, 1162). I'm guessing previous submitter(s) ignored the linting? Edit: Actually I understand why the other lines were changed because the additional indenting pushed them over the edge. Still not sure why it changed this one though. |
| 609 ↗ | (On Diff #4583) | I used the v4.0.1 binary that @schancel provided to me. |
| 1122 ↗ | (On Diff #4583) | Good catch. That's actually a mistake, I failed to reference it on line 1123. |
| 1123 ↗ | (On Diff #4583) | This is supposed to reference blockIndex now instead of nullptr. |