[FIX] [RPC] Align getrawtransaction with Bitcoin Core#985
Open
moisesPompilio wants to merge 4 commits into
Open
[FIX] [RPC] Align getrawtransaction with Bitcoin Core#985moisesPompilio wants to merge 4 commits into
moisesPompilio wants to merge 4 commits into
Conversation
044b3b7 to
6099cc1
Compare
Collaborator
Author
|
This PR is in draft because it depends on #821. That PR introduces typos for spell checking, allowing more flexibility when using randomly generated addresses without triggering spelling errors |
…osity to numeric type The RPC client was incorrectly calling 'gettransaction' (which doesn't exist on the server) instead of 'getrawtransaction'. Additionally, the verbosity parameter handling was inconsistent. While the method signature correctly accepted Option<u32>, it needed better standardization to match Bitcoin Core's RPC specification.
5a99608 to
9547e4a
Compare
…format This commit aligns the getrawtransaction RPC response with Bitcoin Core's format and behavior. The following changes were made: RawTx struct: - Made blockhash, confirmations, blocktime, and time optional fields They are now only serialized when present, avoiding null values - Fixed txid encoding that was previously inverted TxOut struct: - Changed value from u64 (sats) to f64 (BTC) for proper denomination - Made address field optional, only serialized when script can be converted to a valid address TxIn struct: - Added coinbase field that only appears in coinbase transactions, containing the hex-encoded script from scriptSig - Coinbase inputs have: coinbase, sequence, witness (optional) - Regular inputs have: txid, vout, script_sig, sequence, witness (optional) Script handling: - Improved ASM (assembly) format conversion to strictly follow Bitcoin Core rules and formatting standards TxOut types: - Updated to include all script types that Bitcoin Core supports Note: The `desc` field was not implemented due to complex matching rules. This will be added in a future PR.
Add comprehensive integration test comparing Floresta's `getrawtransaction` RPC output with Bitcoin Core for verbosity levels 0 and 1, covering: - Regular transactions (`P2PKH`, `P2WPKH`, `P2SH`, `P2TR types`) - Coinbase transactions - Input/output field validation - Optional field handling New BaseRPC methods: - `get_raw_transaction()` New BitcoinRPC methods: - `create_wallet()` - `get_new_address()` - `generate_block_to_wallet()` - `send_to_address()`
9547e4a to
c9ac410
Compare
c9ac410 to
22e29a2
Compare
Collaborator
Author
|
22e29a2 I rebased and converted the added integration test to the pytest format. |
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
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
Brings Floresta’s
getrawtransactionRPC closer to Bitcoin Core compatibility.Fixes the RPC method call so the client correctly uses
getrawtransactioninstead of the incorrectgettransaction, and standardizesverbosityhandling to match Bitcoin Core’s numeric behavior.The response format was aligned with Bitcoin Core as well:
RawTxfields such asblockhash,confirmations,blocktime, andtimeare now optional and only serialized when present.txidencoding was corrected.TxOut.valuenow usesf64in BTC instead ofu64in sats, matching Bitcoin Core’s denomination.TxOut.addressis now optional and only included when the script can be converted into a valid address.TxInnow handles coinbase inputs properly, including thecoinbasefield only when applicable.TxOutscript types were expanded to cover the script types supported by Bitcoin Core.Note: the
descfield was intentionally left out, since matching Bitcoin Core’s descriptor logic requires additional investigation and a more complex implementation. That can be handled in a future PR.How to verify the changes you have done?
Compare the output of
getrawtransactionbetween Floresta and Bitcoin Core for bothverbosity = 0andverbosity = 1.The integration test added here already covers this behavior and is the best reference for validation.
P2PKH,P2WPKH,P2SH, andP2TR.