rpc: various fixups for dumptxoutset by jamesob · Pull Request #23155 · bitcoin/bitcoin

jamesob · 2021-10-01T14:54:13Z

This is part of the assumeutxo project (parent PR: #15606)

A few fixes to make this RPC actually useful when generating snapshots.

Generate an assumeutxo hash and display it (sort of a bugfix)
Add nchaintx to output (necessary for use in chainparams entry)
Add path of serialized UTXO file to output

src/rpc/blockchain.cpp

DrahtBot · 2021-10-01T17:41:21Z

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#23497 (Add src/node/ and src/wallet/ code to node:: and wallet:: namespaces by ryanofsky)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

test/functional/rpc_dumptxoutset.py

laanwj · 2021-10-27T10:58:50Z

./tinyformat.h:1028:20:   required from ‘tinyformat::detail::FormatListN<sizeof... (Args)> tinyformat::makeFormatList(const Args& ...) [with Args = {int, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, fs::path, fs::path}]’
./tinyformat.h:1064:37:   required from ‘void tinyformat::format(std::ostream&, const char*, const Args& ...) [with Args = {int, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, fs::path, fs::path}; std::ostream = std::basic_ostream<char>]’
./tinyformat.h:1073:11:   required from ‘std::__cxx11::string tinyformat::format(const char*, const Args& ...) [with Args = {int, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, fs::path, fs::path}; std::__cxx11::string = std::__cxx11::basic_string<char>]’
rpc/blockchain.cpp:2623:5:   required from here
./tinyformat.h:543:24: error: use of deleted function ‘void tinyformat::formatValue(std::ostream&, const char*, const char*, int, const T&) [with T = fs::path; std::ostream = std::basic_ostream<char>]’
             formatValue(out, fmtBegin, fmtEnd, ntrunc, *static_cast<const T*>(value));
             ~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from ./rpc/blockchain.h:10,
                 from rpc/blockchain.cpp:6:
./fs.h:233:24: note: declared here
 template<> inline void formatValue(std::ostream&, const char*, const char*, int, const fs::path&) = delete;
                        ^~~~~~~~~~~

needs to be updated for #22937

jamesob · 2021-10-29T13:56:09Z

needs to be updated for #22937

Fixed, thanks.

jamesob · 2021-11-03T12:49:11Z

This change is functionally tested, should be uncontroversial, easy to review/merge, etc. if anyone has spare cycles.

src/rpc/blockchain.cpp

ryanofsky

Code review ACK b637ffa. Seems like there are some small fixes that could be made here, but this already does seem like an improvement.

src/rpc/blockchain.cpp

src/rpc/blockchain.h

jamesob · 2021-11-16T15:10:11Z

Addressed all feedback - thanks for the good reviews @MarcoFalke @ryanofsky.

src/rpc/blockchain.cpp

9b575f1 Improve fs::PathToString documentation (Russell Yanofsky) Pull request description: Add a developer note about avoiding `fs::PathToString` in RPCs, and improve some other `fs::PathToString` comments. Developer note might have been useful in two recent review comments: - bitcoin/bitcoin#23398 (comment) - bitcoin/bitcoin#23155 (comment) ACKs for top commit: laanwj: Documentation review ACK 9b575f1 jamesob: ACK bitcoin/bitcoin@9b575f1 prayank23: ACK bitcoin/bitcoin@9b575f1 hebasto: ACK 9b575f1 shaavan: ACK 9b575f1 Tree-SHA512: b8b3ecb6208c3897241e4f24dcec64fe7cf091bc79388862cf5f4b315cb8e804939981c4bed4c81dbff99ec9f750bad99015d0f04890704ac9df63c2a6719b6d

ryanofsky

Code review ACK 7c27d38ccc89d49a7271b3d9a43159b34f8baed1. Just a few suggested tweaks since last review for casting and argument passing and RPC documentation.

src/rpc/blockchain.cpp

9b575f1 Improve fs::PathToString documentation (Russell Yanofsky) Pull request description: Add a developer note about avoiding `fs::PathToString` in RPCs, and improve some other `fs::PathToString` comments. Developer note might have been useful in two recent review comments: - bitcoin#23398 (comment) - bitcoin#23155 (comment) ACKs for top commit: laanwj: Documentation review ACK 9b575f1 jamesob: ACK bitcoin@9b575f1 prayank23: ACK bitcoin@9b575f1 hebasto: ACK 9b575f1 shaavan: ACK 9b575f1 Tree-SHA512: b8b3ecb6208c3897241e4f24dcec64fe7cf091bc79388862cf5f4b315cb8e804939981c4bed4c81dbff99ec9f750bad99015d0f04890704ac9df63c2a6719b6d

- Actually generate an assumeutxo hash and display it - Add nchaintx to output (necessary for use in chainparams entry) - Add path of serialized UTXO file to output

jamesob · 2021-11-30T16:22:50Z

Pushed a small doc fix based on Marco, Russ' feedback.

I think this small change is easily reviewable and could be merged without much risk.

laanwj · 2021-12-02T09:26:18Z

Code review ACK ffd0928

026443b rpc: various fixups for dumptxoutset (James O'Beirne) Pull request description: This is part of the [assumeutxo project](https://github.com/bitcoin/bitcoin/projects/11) (parent PR: #15606) --- A few fixes to make this RPC actually useful when generating snapshots. - Generate an assumeutxo hash and display it (sort of a bugfix) - Add nchaintx to output (necessary for use in chainparams entry) - Add path of serialized UTXO file to output ACKs for top commit: laanwj: Code review ACK 026443b Tree-SHA512: b0b5fd5138dea0e21258b1b18ab75bf3fd1628522cc1dbafa81af9cb9fa96562a1c39124fdb31057f256bfc560f462f907e9fe5e209b577b3f57afae2b7be826

DrahtBot added the RPC/REST/ZMQ label Oct 1, 2021

benthecarman reviewed Oct 1, 2021

View reviewed changes

src/rpc/blockchain.cpp Outdated Show resolved Hide resolved

jamesob force-pushed the 2021-10-au-rpc-fixes branch from 0b3ed75 to 1e161db Compare October 1, 2021 18:27

This was referenced Oct 1, 2021

consensus: move amount.h into consensus #22951

Merged

rpc: Add level 3 verbosity to getblock RPC call (#21245 modified) #22918

Merged

benthecarman reviewed Oct 1, 2021

View reviewed changes

test/functional/rpc_dumptxoutset.py Outdated Show resolved Hide resolved

This was referenced Oct 2, 2021

rpc: getblockfrompeer #20295

Merged

assumeutxo #15606

Closed

DrahtBot added the Needs rebase label Oct 5, 2021

jamesob force-pushed the 2021-10-au-rpc-fixes branch from 1e161db to 9b0c162 Compare October 26, 2021 13:11

DrahtBot removed the Needs rebase label Oct 26, 2021

jamesob force-pushed the 2021-10-au-rpc-fixes branch 2 times, most recently from bda4d42 to 7d6a994 Compare October 27, 2021 14:46

jamesob force-pushed the 2021-10-au-rpc-fixes branch 2 times, most recently from 5b542e5 to b637ffa Compare October 29, 2021 13:59

DrahtBot mentioned this pull request Nov 1, 2021

refactor: Avoid integer overflow in ApplyStats when activating snapshot #23411

Merged

maflcko reviewed Nov 3, 2021

View reviewed changes

src/rpc/blockchain.cpp Outdated Show resolved Hide resolved

src/rpc/blockchain.cpp Outdated Show resolved Hide resolved

DrahtBot mentioned this pull request Nov 13, 2021

Add src/node/ and src/wallet/ code to node:: and wallet:: namespaces #23497

Merged

ryanofsky approved these changes Nov 16, 2021

View reviewed changes

src/rpc/blockchain.cpp Outdated Show resolved Hide resolved

src/rpc/blockchain.cpp Outdated Show resolved Hide resolved

src/rpc/blockchain.h Outdated Show resolved Hide resolved

ryanofsky mentioned this pull request Nov 16, 2021

Improve fs::PathToString documentation #23522

Merged

jamesob force-pushed the 2021-10-au-rpc-fixes branch from b637ffa to 1299ae9 Compare November 16, 2021 15:09

maflcko reviewed Nov 16, 2021

View reviewed changes

src/rpc/blockchain.cpp Outdated Show resolved Hide resolved

jamesob force-pushed the 2021-10-au-rpc-fixes branch 2 times, most recently from 6bf0fd2 to 7c27d38 Compare November 16, 2021 15:22

ryanofsky approved these changes Nov 17, 2021

View reviewed changes

src/rpc/blockchain.cpp Outdated Show resolved Hide resolved

rpc: various fixups for dumptxoutset

ffd0928

- Actually generate an assumeutxo hash and display it - Add nchaintx to output (necessary for use in chainparams entry) - Add path of serialized UTXO file to output

jamesob force-pushed the 2021-10-au-rpc-fixes branch from 7c27d38 to ffd0928 Compare November 30, 2021 16:19

laanwj merged commit 6acda4b into bitcoin:master Dec 2, 2021

sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Dec 2, 2021

Merge bitcoin#23155: rpc: various fixups for dumptxoutset

4a27cee

bitcoin locked and limited conversation to collaborators Dec 2, 2022

Conversation

jamesob commented Oct 1, 2021

Uh oh!

Uh oh!

DrahtBot commented Oct 1, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Conflicts

Uh oh!

Uh oh!

laanwj commented Oct 27, 2021

Uh oh!

jamesob commented Oct 29, 2021

Uh oh!

jamesob commented Nov 3, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ryanofsky left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jamesob commented Nov 16, 2021

Uh oh!

Uh oh!

ryanofsky left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jamesob commented Nov 30, 2021

Uh oh!

laanwj commented Dec 2, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

DrahtBot commented Oct 1, 2021 •

edited

Loading

jamesob commented Nov 3, 2021 •

edited

Loading