rpc: allow dumptxoutset to dump human-readable data#24202
rpc: allow dumptxoutset to dump human-readable data#24202w0xlt wants to merge 2 commits intobitcoin:masterfrom
Conversation
w0xlt
force-pushed
the
feature-utxo-ascii
branch
from
11d03f7 to
e89029e
Compare
w0xlt
force-pushed
the
feature-utxo-ascii
branch
2 times, most recently
from
8426bda to
4ad72ef
Compare
shaavan
left a comment
shaavan
left a comment
There was a problem hiding this comment.
Concept ACK
I like the idea of allowing the option to dumptxoutset in human-readable data. Also, at first glance, the code looks very clean. Great work! @w0xlt.
I shall post my complete review after thoroughly reviewing this PR. In the meantime, I found some nit suggestions that might help make this PR even better.
w0xlt
force-pushed
the
feature-utxo-ascii
branch
from
53c1662 to
4cef616
Compare
brunoerg
left a comment
brunoerg
left a comment
There was a problem hiding this comment.
tACK 4cef616
(MacOS 12)
./src/bitcoin-cli dumptxoutset utxo.txt true
{
"coins_written": 40495,
"base_hash": "000000002cb3254c66a5510fe4b921d5e5d72e85b5111455365f519dee915ce3",
"base_height": 48205,
"path": "/Users/brunogarcia/Library/Application Support/Bitcoin/utxo.txt",
"txoutset_hash": "a4d1ef75d84b1dbda8df305661ef8e8bbcc3fc86e12e4d425255eaf04030100c",
"nchaintx": 48821
}
I checked the file content and seems right.
Ran again and got:
error code: -8
error message:
/Users/brunogarcia/Library/Application Support/Bitcoin/utxo.txt already exists. If you are sure this is what you want, move it out of the way first
Co-authored-by: Shashwat Vangani <[email protected]> Co-authored-by: Luke Dashjr <[email protected]>
Co-authored-by: Luke Dashjr <[email protected]> Co-authored-by: brunoerg <[email protected]>
w0xlt
force-pushed
the
feature-utxo-ascii
branch
from
e7b3b6d to
1053636
Compare
|
@luke-jr , I accepted your suggestions. While I personally prefer simpler RPC (something like I tried merging your commits first, but when I dropped the previous commits, this caused a lot of conflicts. So I reset and re-commit. If you prefer another approach, let me know. |
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsReviewers, this pull request conflicts with the following ones:
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. |
|
Concept ACK I think on the long-term writing a database (e.g. sqlite) would be even more helpful, as a text file in the size of multiple giga-bytes is relatively hard to handle without an index. But this is obviously out-of-scope for this PR and probably can also be solved by a script that creates the database with the human-readable data as input. |
|
🐙 This pull request conflicts with the target branch and needs rebase. Want to unsubscribe from rebase notifications on this pull request? Just convert this pull request to a "draft". |
There hasn't been much activity lately and the patch still needs rebase. What is the status here?
|
|
Closing this as it has not had any activity in a while. If you are interested in continuing work on this, please leave a comment so that it can be reopened. |
8 participants
As #18689 is labeled "Up for grabs" , this PR proposes a modified version of it.
The changes (from the original) are :
. RPC has been simplified. It now has only one additional parameter (
human_readable=true/false).. The
CreateUTXOSnapshotsignature has been simplified, with reduced number of parameters andcoinascii_cb_tdata type has been removed.. The functional test verifies the content of the human-readable file.
The generated file, however, is the same.
To test:
./src/bitcoin-cli dumptxoutset utxo.txt true