fix: inconsitencies on gettxoutproof#1113
Open
jaoleal wants to merge 5 commits into
Open
Conversation
Member
Author
jaoleal
force-pushed
the
fix/gettxoutproof_apiinconsistency_docs_tests
branch
2 times, most recently
from
7ba095f to
1b3c8c8
Compare
Davidson-Souza
added
the
reliability
jaoleal
force-pushed
the
fix/gettxoutproof_apiinconsistency_docs_tests
branch
from
1b3c8c8 to
ac6150e
Compare
Member
|
In 8f0d7a5:
Accidentally what? |
Change GetTxOutProof to hold a String internally and hex-encode the merkle proof bytes in blockchain.rs before constructing the type. This ensures serde_json::to_value() naturally produces the correct hex string.
The FlorestaRPC trait defined get_txout_proof as the only method returning Option<String> instead of Result<T>. The implementation used .ok() to silently discard all errors from self.call() — connection errors, RPC errors, TxNotFound, etc. — causing the CLI to always print "null" instead of meaningful error messages. Change the trait signature to Result<String>, remove the .ok() call, and add ? to the CLI dispatch so errors propagate to the user.
jaoleal
force-pushed
the
fix/gettxoutproof_apiinconsistency_docs_tests
branch
from
ac6150e to
e5d54a5
Compare
Member
Author
|
Fixed @Davidson-Souza |
jaoleal
requested review from
Davidson-Souza and
csgui
and removed request for
Davidson-Souza
jaoleal
force-pushed
the
fix/gettxoutproof_apiinconsistency_docs_tests
branch
from
e5d54a5 to
48e97d3
Compare
Member
Author
|
Without the error-context clap flag, clap uses ErrorKind::as_str() as a static fallback with no argument name, invalid value, or error detail from the parser. This commit also deletes the parsers.rs and in its place we use a simple closure that allow us to use serde_json::from_str
Add a functional test that syncs a three-node network (florestad, bitcoind, utreexod), then compares Merkle proofs returned by gettxoutproof between Floresta and Bitcoin Core for each transaction in blocks 2 through 9. Also add get_txout_proof() helper to the BaseRPC test framework class.
jaoleal
force-pushed
the
fix/gettxoutproof_apiinconsistency_docs_tests
branch
from
48e97d3 to
1cd9af7
Compare
Member
Author
|
We can use a closure instead GetTxOutProof {
/// The transaction IDs to prove
#[arg(required = true, value_parser = |s: &str| serde_json::from_str::<Vec<Txid>>(s).map_err(|e| e.to_string()))]
txids: std::vec::Vec<Txid>, // you need to specify the path of Vec https://github.com/clap-rs/clap/discussions/4695 |
| """ | ||
| return self.perform_request("generatetoaddress", params=[nblocks, address]) | ||
|
|
||
| def send_to_address(self, address: str, amount: float) -> str: |
Member
There was a problem hiding this comment.
You didn't use this. It would be nice to have a few transactions inside a block to test
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description and Notes
This pr fix some inconsistencies focusing on gettxoutproof;
.ok().Each of these is being fixed in a commit.
Depends on #831