rpc: gettxoutsetinfo can be requested for specific blockheights
ClosedPublic
Actions

Authored by PiRK on Jun 9 2022, 16:15.

Details

Reviewers

Fabien

Group Reviewers

Restricted Project

Commits

rABC031b81d9f7ab: rpc: gettxoutsetinfo can be requested for specific blockheights

Summary

This is a backport of core#19521 [6 & 7/17]
https://github.com/bitcoin/bitcoin/pull/19521/commits/3f166ecc125fce6ccd995687fa16572090a5d099
https://github.com/bitcoin/bitcoin/pull/19521/commits/6a4c0c09ab4d073a26c3c4a02783d5dcd88f6eef (removal of 'disk_size' and 'transactions' from the index and test the RPC on specific heights)

Notes:

in the source commits there is a call to ::ChainstateActive().ForceFlushStateToDisk(); added, but it is removed in a following commit in the same PR. This is obviously duplicated with the active_chainstate.ForceFlushStateToDisk() a few lines later, so I omitted it.
I added a mention about disk_size not being available when using the index in the doc of the RPC call. This is added later in the same PR in an unrelated commit in the source material.
the new hash_or_height RPC parameter needs to be added to the CRPCConvertParamtable for the unit tests to pass. This is done in a later commit in the same PR in the source material

Depends on D11599

Test Plan

ninja all check-all

Diff Detail

Repository

rABC Bitcoin ABC

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 19368
Build 38472: Build Diff	build-debug · lint-circular-dependencies · build-diff · build-without-wallet · build-clang · build-clang-tidy
Build 38471: arc lint + arc unit

Event Timeline

PiRK created this revision.Jun 9 2022, 16:15

Herald added a reviewer: Restricted Project. · View Herald TranscriptJun 9 2022, 16:15

PiRK requested review of this revision.Jun 9 2022, 16:15

teamcity edited the summary of this revision. (Show Details)Jun 9 2022, 16:16

Harbormaster completed remote builds in B19330: Diff 33929.Jun 9 2022, 16:38

Build Bitcoin ABC Diffs / Diff Testing (build-clang-tidy) failed.

Tail of the build log:

[340/518] Building CXX object src/CMakeFiles/bitcoinconsensus.dir/pubkey.cpp.o
[341/518] Building CXX object src/CMakeFiles/server.dir/rpc/blockchain.cpp.o
[342/518] Building C object src/secp256k1/CMakeFiles/secp256k1.dir/src/secp256k1.c.o
[343/518] Linking C static library src/secp256k1/libsecp256k1.a
[344/518] Linking C executable src/secp256k1/ecmult-bench
[345/518] Linking C executable src/secp256k1/internal-bench
[346/518] Building CXX object src/CMakeFiles/bitcoinconsensus.dir/consensus/tx_check.cpp.o
[347/518] Linking C executable src/secp256k1/sign-bench
[348/518] Linking CXX static library src/libbitcoinconsensus.a
[349/518] Building CXX object src/CMakeFiles/server.dir/net_processing.cpp.o
[350/518] Linking C executable src/secp256k1/verify-bench
[351/518] Linking CXX static library src/libscript.a
[352/518] Linking C executable src/secp256k1/recover-bench
[353/518] Building CXX object src/test/CMakeFiles/testutil.dir/util/str.cpp.o
[354/518] Building CXX object src/test/CMakeFiles/testutil.dir/util/logging.cpp.o
[355/518] Building CXX object src/CMakeFiles/common.dir/rpc/rawtransaction_util.cpp.o
[356/518] Building CXX object src/test/CMakeFiles/testutil.dir/util/transaction_utils.cpp.o
[357/518] Building CXX object src/CMakeFiles/bitcoin-wallet.dir/bitcoin-wallet.cpp.o
[358/518] Building CXX object src/CMakeFiles/bitcoin-cli.dir/bitcoin-cli.cpp.o
[359/518] Building CXX object src/wallet/CMakeFiles/wallet.dir/context.cpp.o
[360/518] Building CXX object src/CMakeFiles/common.dir/rpc/util.cpp.o
[361/518] Building CXX object src/wallet/CMakeFiles/wallet.dir/coincontrol.cpp.o
[362/518] Linking CXX static library src/libcommon.a
[363/518] Linking CXX shared library src/libbitcoinconsensus.so.0.25.8
[364/518] Creating library symlink src/libbitcoinconsensus.so.0 src/libbitcoinconsensus.so
[365/518] Linking CXX executable src/bitcoin-cli
[366/518] Building CXX object src/wallet/CMakeFiles/wallet.dir/coinselection.cpp.o
[367/518] Building CXX object src/test/CMakeFiles/testutil.dir/util/blockfilter.cpp.o
[368/518] Building CXX object src/wallet/CMakeFiles/wallet.dir/crypter.cpp.o
[369/518] Building CXX object src/test/CMakeFiles/testutil.dir/util/mining.cpp.o
[370/518] Building CXX object src/test/CMakeFiles/testutil.dir/util/net.cpp.o
[371/518] Building CXX object src/test/CMakeFiles/testutil.dir/util/wallet.cpp.o
[372/518] Building CXX object src/wallet/CMakeFiles/wallet.dir/db.cpp.o
[373/518] Building CXX object src/wallet/CMakeFiles/wallet.dir/bdb.cpp.o
[374/518] Building CXX object src/CMakeFiles/bitcoin-tx.dir/bitcoin-tx.cpp.o
[375/518] Linking CXX executable src/bitcoin-tx
[376/518] Building CXX object src/CMakeFiles/server.dir/validation.cpp.o
[377/518] Building CXX object src/test/CMakeFiles/testutil.dir/util/setup_common.cpp.o
[378/518] Building CXX object src/wallet/CMakeFiles/wallet.dir/walletutil.cpp.o
[379/518] Building CXX object src/wallet/CMakeFiles/wallet-tool.dir/wallettool.cpp.o
[380/518] Building CXX object src/seeder/CMakeFiles/seeder-base.dir/dns.cpp.o
[381/518] Building CXX object src/seeder/CMakeFiles/seeder-base.dir/db.cpp.o
[382/518] Building CXX object src/wallet/CMakeFiles/wallet.dir/sqlite.cpp.o
[383/518] Building CXX object src/wallet/CMakeFiles/wallet.dir/fees.cpp.o
[384/518] Building CXX object src/wallet/CMakeFiles/wallet.dir/load.cpp.o
[385/518] Building CXX object src/seeder/CMakeFiles/seeder-base.dir/options.cpp.o
[386/518] Building CXX object src/wallet/CMakeFiles/wallet.dir/__/interfaces/wallet.cpp.o
[387/518] Building CXX object src/wallet/CMakeFiles/wallet.dir/salvage.cpp.o
[388/518] Building CXX object src/seeder/CMakeFiles/seeder-base.dir/bitcoin.cpp.o
[389/518] Building CXX object src/seeder/CMakeFiles/bitcoin-seeder.dir/main.cpp.o
[390/518] Building CXX object src/wallet/CMakeFiles/wallet.dir/scriptpubkeyman.cpp.o
[391/518] Building CXX object src/wallet/CMakeFiles/wallet.dir/walletdb.cpp.o
[392/518] Building CXX object src/wallet/CMakeFiles/wallet.dir/rpcdump.cpp.o
[393/518] Building CXX object src/wallet/CMakeFiles/wallet.dir/wallet.cpp.o
[394/518] Building CXX object src/wallet/CMakeFiles/wallet.dir/rpcwallet.cpp.o
[395/518] Linking CXX static library src/wallet/libwallet.a
[396/518] Linking CXX static library src/wallet/libwallet-tool.a
[397/518] Linking CXX executable src/bitcoin-wallet
ninja: build stopped: cannot make progress due to previous errors.
Build build-clang-tidy failed with exit code 1

PiRK added a child revision: D11604: rpc: Add Coinstats index to getindexinfo.Jun 10 2022, 08:31

rebase

Harbormaster completed remote builds in B19342: Diff 33949.Jun 10 2022, 09:43

PiRK mentioned this in D11611: doc: coinstatsindex related touchups.Jun 10 2022, 10:04

Fabien requested changes to this revision.Jun 10 2022, 22:59

Fabien added a subscriber: Fabien.

Fabien added inline comments.

src/rpc/blockchain.cpp
118 ↗	(On Diff #33949)	static
1309 ↗	(On Diff #33949)	Now it's duplicated
1328 ↗	(On Diff #33949)	That doesn't come from any of the linked commits
1349 ↗	(On Diff #33949)	There is a call to `::ChainstateActive().ForceFlushStateToDisk();` in the source material
src/rpc/client.cpp
114 ↗	(On Diff #33949)	Is that necessary ?

This revision now requires changes to proceed.Jun 10 2022, 22:59

PiRK added inline comments.Jun 11 2022, 12:13

src/rpc/blockchain.cpp
1328 ↗	(On Diff #33949)	It is added later in the same PR in an unrelated commit, it logically belongs here.
1349 ↗	(On Diff #33949)	It is later removed in a following commit with no explanation. I'm assuming that it is duplicated by mistake, as there is another `active_chainstate.ForceFlushStateToDisk()` call in a few lines (see line 1360). I will mention this in the summary.
src/rpc/client.cpp
114 ↗	(On Diff #33949)	Yes. Otherwise there is failing test. The source material adds this in one of the following commits.

PiRK edited the summary of this revision. (Show Details)Jun 11 2022, 12:16

PiRK edited the summary of this revision. (Show Details)Jun 11 2022, 12:24

Make the ParseHashOrHeight function static.
Remove duplicated "transactions" parameter in the RPC help

Add more information in the summary about deviations from the source material. Those are all fixups that come from following commits from the same PR but logically belong to this commit.

Harbormaster completed remote builds in B19368: Diff 33980.Jun 11 2022, 12:42

Fabien accepted this revision.Jun 12 2022, 07:32

This revision is now accepted and ready to land.Jun 12 2022, 07:32

Closed by commit rABC031b81d9f7ab: rpc: gettxoutsetinfo can be requested for specific blockheights (authored by Fabian Jahr <fjahr@protonmail.com>, committed by PiRK). · Explain WhyJun 13 2022, 18:28

This revision was automatically updated to reflect the committed changes.

PiRK added a commit: rABC031b81d9f7ab: rpc: gettxoutsetinfo can be requested for specific blockheights.

PiRK mentioned this in rABC80020c9a97cb: doc: coinstatsindex related touchups.