This commit adds the final block hash of the window to getchaintxstats
and documents how reviewers can verify changes to chainTxData.
Backport of PR12317: https://github.com/bitcoin/bitcoin/pull/12317/
Completes T566.
Details
Details
- Reviewers
jasonbcox deadalnix Fabien - Group Reviewers
Restricted Owners Package (Owns No Changed Paths) Restricted Project - Commits
- rSTAGING129395a132a3: Document method for reviewers to verify chainTxData
rABC129395a132a3: Document method for reviewers to verify chainTxData
make check
test_runner.py rpc_blockchain
Diff Detail
Diff Detail
- Repository
- rABC Bitcoin ABC
- Branch
- BackportPR12317
- Lint
Lint Passed - Unit
No Test Coverage - Build Status
Buildable 5366 Build 8794: Bitcoin ABC Buildbot (legacy) Build 8793: arc lint + arc unit
Event Timeline
Comment Actions
The new return value from the getchaintxstats RPC may deserve an entry in the release notes.
This also raises a few questions:
- Do we want to update these params every release or only on major ones like bitcoin core do ?
- The current tx rate param does not reflect the actual tx rate, but according to getchaintxstats it doesn't seem to match the other actual params either (./src/bitcoin-cli getchaintxstats 4320 000000000000000001d2ce557406b017a928be25ee98906397d339c3f68eec5d returns tx rate of 0.23). Is this intentional ? (And is it the correct window ?)
| doc/release-process.md | ||
|---|---|---|
| 30 ↗ | (On Diff #7751) | I think you must backport PR12309 first, this will add some context to this sentence: https://github.com/bitcoin/bitcoin/pull/12309 |
| src/rpc/blockchain.cpp | ||
| 1736 ↗ | (On Diff #7751) | Only porting this line is breaking the help output indent (you can see this through ./src/bitcoin-cli help getchaintxstats). |
| 1809 ↗ | (On Diff #7751) | Use pushKV instead: |
| doc/release-process.md | ||
|---|---|---|
| 30 ↗ | (On Diff #7751) | This also needs to be indented since it's in the chainparams section. |
| doc/release-process.md | ||
|---|---|---|
| 30 ↗ | (On Diff #7751) | @Fabien I actually reworded the line as it was in this PR to better reflect our update process. As far as I'm aware, we don't update transaction rate or count in chainparams.cpp which makes PR12309's first line irrelevant. I didn't include the second line change in 12309 for similar reasons. @jasonbcox This line should be indented to be in line with line 29? Should the next line (31) be indented past that or the same? |
| doc/release-process.md | ||
|---|---|---|
| 30 ↗ | (On Diff #7751) | Nvm. The above block only refers to updating chainparams' assume valid and chain work. It's fine as-is. |
Comment Actions
I'm not sure if this should be part of our release process.
| doc/release-process.md | ||
|---|---|---|
| 30 ↗ | (On Diff #7751) | The more I look into this, the less valuable it looks to me. The only use of chainTxData is for guessing verification progress in the wallet. @deadalnix do you see value in bloating our release process to add updating chainTxData? I don't. |
Comment Actions
Updating the txrate can be added later to the release process with no drawback, and I agree with Jason that it has low value.