[FIX] [RPC] Align getrawtransaction with Bitcoin Core by moisesPompilio · Pull Request #985 · getfloresta/Floresta

moisesPompilio · 2026-04-23T02:35:16Z

Description and Notes

Brings Floresta’s getrawtransaction RPC closer to Bitcoin Core compatibility.

Fixes the RPC method call so the client correctly uses getrawtransaction instead of the incorrect gettransaction, and standardizes verbosity handling to match Bitcoin Core’s numeric behavior.

The response format was aligned with Bitcoin Core as well:

RawTx fields such as blockhash, confirmations, blocktime, and time are now optional and only serialized when present.
txid encoding was corrected.
TxOut.value now uses f64 in BTC instead of u64 in sats, matching Bitcoin Core’s denomination.
TxOut.address is now optional and only included when the script can be converted into a valid address.
TxIn now handles coinbase inputs properly, including the coinbase field only when applicable.
ASM formatting was improved to follow Bitcoin Core’s rules more strictly.
TxOut script types were expanded to cover the script types supported by Bitcoin Core.

Note: the desc field 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 getrawtransaction between Floresta and Bitcoin Core for both verbosity = 0 and verbosity = 1.

The integration test added here already covers this behavior and is the best reference for validation.

Check regular transactions across multiple script types, including P2PKH, P2WPKH, P2SH, and P2TR.
Check coinbase transactions and confirm the special input/output fields are handled correctly.
Review the optional field behavior to ensure fields are only present when they should be.
Inspect the integration test to confirm field-by-field validation against Bitcoin Core.

moisesPompilio · 2026-04-23T02:41:21Z

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

moisesPompilio · 2026-05-15T02:20:16Z

22e29a2 I rebased and converted the added integration test to the pytest format.

jaoleal · 2026-05-15T14:30:31Z

-    #[command(name = "gettransaction")]
-    GetTransaction { txid: Txid, verbose: Option<bool> },
+    #[command(name = "getrawtransaction")]
+    GetRawTransaction { txid: Txid, verbose: Option<u32> },


I dont think that Bitcoin Core will appear with 4 294 967 295 levels of verbosity

I agree, but u32 is the default for numeric verbosity that we use.

u32 occupies too much space for something simple as a verbosity level.

Also: #1076

I replaced the u32 with a u8.

jaoleal · 2026-05-15T14:31:17Z

+        match verbosity {
+            0 => serde_json::to_value(serialize_hex(&tx.tx))
+                .map_err(|e| JsonRpcError::Decode(e.to_string())),
+            1 => serde_json::to_value(self.make_raw_transaction(tx))
+                .map_err(|e| JsonRpcError::Decode(e.to_string())),
+            _ => return Err(JsonRpcError::InvalidVerbosityLevel),
+        }
    }


Return a structured type instead of a generic

jaoleal · 2026-05-15T14:41:01Z


+/// The information returned by a get_raw_tx
 #[derive(Deserialize, Serialize)]
 pub struct RawTxJson {


Did you tried to use corepc ?

Because the GetRawTransaction of corepc is not good, the coinbase field of the input is missing, and in addition, old fields are present.

Davidson-Souza · 2026-05-20T13:16:48Z

nit: the bang (!) should come before the column (:) in your commit titles. fix(rpc)!:

Davidson-Souza

first code review

Davidson-Souza · 2026-05-20T13:21:12Z

+        if verbosity > 1 {
+            return Err(JsonRpcError::InvalidVerbosityLevel);
        }


03bae40

nit: Since there's a wildcard inside the match below, this check is useless.

Yeah, I didn’t want the verbosity check to be after getting the transaction, but it doesn’t really matter, since it would only save a few milliseconds in the process. So I removed this if; the validation is in the match now.

Davidson-Souza · 2026-05-20T13:24:56Z

+pub enum GetRawTransactionRes {
+    Zero(String),
+
+    One(Box<RawTxJson>),
+}


b07a232

nit:

Suggested change

pub enum GetRawTransactionRes {

Zero(String),

One(Box<RawTxJson>),

}

pub enum GetRawTransactionRes {

Zero(String),

One(Box<RawTxJson>),

}

Also, you Box-ed it because it is too big?

Yes, we are adopting Box for the response enums because the verbosity objects tend to be very large.

Davidson-Souza · 2026-05-20T15:54:10Z

+}
+
+/// The information returned by a get_raw_tx
+#[derive(Debug, Deserialize, Serialize)]


b07a232

Suggested change

#[derive(Debug, Deserialize, Serialize)]

#[derive(Debug, Deserialize, Serialize)]

/// The information returned by a get_raw_tx

jaoleal

ACK 7331f65

Davidson-Souza · 2026-06-02T15:36:14Z

Needs rebase

moisesPompilio · 2026-06-04T14:56:18Z

d0b8750 I’ve rebased

Davidson-Souza · 2026-06-09T16:17:58Z

-    #[command(name = "gettransaction")]
-    GetTransaction { txid: Txid, verbose: Option<bool> },
+    #[command(name = "getrawtransaction")]
+    GetRawTransaction { txid: Txid, verbose: Option<u8> },


verbosity (in other places as well).

Davidson-Souza · 2026-06-09T16:19:35Z

+                let raw_tx = self.make_raw_transaction(tx)?;
+                serde_json::to_value(raw_tx).map_err(|e| JsonRpcError::Decode(e.to_string()))
+            }
+            _ => Err(JsonRpcError::InvalidVerbosityLevel),


I think core always return the maximum known level if the number isn't defined — if I pass 3, it returns the same as 1

No, in getrawtransaction, Bitcoin Core returns 0 as the default value, according to the documentation.
https://bitcoincore.org/en/doc/30.0.0/rpc/rawtransactions/getrawtransaction/

Davidson-Souza · 2026-06-09T16:23:28Z

+}
+
+#[derive(Debug, Deserialize, Serialize)]
+/// The information returned by a get_raw_tx


Doesn't corepc have those types?

yes, but the GetRawTransaction of corepc is not good, the coinbase field of the input is missing, and in addition, old fields are present.

Perhaps one an issue and leave a TODO here?

I checked the newer versions of corepc, and the getrawtransaction structure has been improved. So I updated to the latest version and used its struct here.

Davidson-Souza · 2026-06-09T16:24:20Z

+    /// The weight of this transaction, as defined by the segwit soft-fork
    pub weight: u32,
+
+    /// This transaction's version. The current bigger version is 2


Actually, three (TRUC transactions)

Davidson-Souza · 2026-06-09T16:25:50Z

+#[derive(Debug, Deserialize, Serialize)]
+/// The locking script inside a txout
 pub struct ScriptPubKeyJson {
+    /// A ASM representation for this script


Davidson-Souza · 2026-06-09T16:28:10Z

                .witness
                .iter()
-                .map(|w| w.to_hex_string(bitcoin::hex::Case::Upper))
+                .map(|w| w.to_hex_string(bitcoin::hex::Case::Lower))


hex::Case::Lower

Davidson-Souza · 2026-06-09T16:29:50Z

-
+    /// TODO: This function is not compliant with Bitcoin Core.
+    /// See: <https://github.com/getfloresta/Floresta/issues/987>
+    pub(super) fn get_script_type_descriptor(script: &Script, address: &Option<Address>) -> String {


These methods could be an extension trait for Address or Script

Yes, I’m planning to work on that in issue #1108, so this will be a separate PR to add extensions for transaction items, since some parts are shared between floresta-node and floresta-electrum.

Davidson-Souza · 2026-06-09T16:33:19Z

+        block = self.bitcoind.rpc.get_block(bestblockhash)
+        txids.append(block["tx"][0])  # Get the coinbase transaction txid
+
+        self.utreexod = utreexod_node


In this case, Utreexo is only used here to provide transaction proofs to Floresta, so it basically just connects to other peers and performs the IBD. That’s why it is only called in this lower part.

But why the bind?

Right, I removed it.

moisesPompilio · 2026-06-17T22:21:02Z

b99a917 Added a helper function to compare fields, which helps in cases where we need to compare responses against Bitcoin Core. Also rebased the branch.

jaoleal · 2026-06-19T15:59:54Z

@moisesPompilio CI broke

jaoleal

Some suggestions

jaoleal · 2026-06-22T14:15:05Z

+        with pytest.raises(HTTPError):
+            self.florestad.rpc.get_raw_transaction("nonexistingtxid")
+
+        with pytest.raises(HTTPError):
+            self.florestad.rpc.get_raw_transaction("nonexistingtxid", verbose=0)
+
+        with pytest.raises(HTTPError):
+            self.florestad.rpc.get_raw_transaction("nonexistingtxid", verbose=1)


Now we have tooling for the framework to precisely assert against rpc errors, dont you wanna use it here ?

jaoleal · 2026-06-22T14:18:57Z

+
+        txids = []
+        value = 6.4242521
+        for address in [


I see other tests that can use a chain with transactions, perhaps we could extract this into a shared helper ? Same thing we did with the wait_until wrapper

Hmm, I think it's better to leave this for later. I'd like this helper function to be implemented using Floresta's wallet functionality rather than Bitcoin Core's

jaoleal · 2026-06-22T14:23:30Z

    raise TimeoutError(f"{error_msg} after {timeout} seconds")
+
+
+def compare_fields(candidate, reference, ignore_fields=None, float_tol=1e-8):


jaoleal · 2026-06-22T14:26:47Z

+        for key, ref_value in reference.items():
+            if key in ignore_fields:
+                continue
+            assert key in candidate, f"Missing key in candidate: {key}"
+            compare_fields(candidate[key], ref_value, ignore_fields=ignore_fields)


THis method doesnt need to be recursive but okay

Actually, it is necessary. We have cases with dicts nested inside other dicts, as well as dicts inside lists that are themselves nested within dicts. That's why the recursion is needed. A direct comparison would not work for those cases.

jaoleal · 2026-06-22T14:28:33Z

+        return
+
+    # scalar
+    assert candidate == reference


This have the "failfast" behavior right ? itll stop comparing at the first difference, dont we want to know all fields that differ (in a single run)?

Yes, this is fail-fast because it relies on assertions. As soon as a comparison fails, the entire comparison chain stops.

In this case, collecting all differences is not really feasible because the function is generic and does not know the structure of the data beforehand. For example, a getrawtransaction response contains a top-level dict, a vin field that is a list, and each item in that list is another dict. Since the structure can vary depending on the RPC response being compared, the function recursively traverses the data and fails as soon as it encounters the first mismatch.

jaoleal · 2026-06-22T14:29:54Z

+        for height in range(initial_block_coinbase_wallet, best_block_height):
+            block_hash = self.bitcoind.rpc.get_blockhash(height)
+
+            block = self.bitcoind.rpc.get_block(block_hash, verbosity=1)
+            coinbase_tx = block["tx"][0]  # Get the coinbase transaction txid
+
+            self.compare_getrawtransaction(coinbase_tx)


You are checking coinbases twice ?

jaoleal · 2026-06-22T14:32:18Z

+- `InvalidVerbosityLevel` - Verbosity parameter is not 0 or 1
+- `MissingParameter` - The txid parameter was not provided
+- `InvalidParameterType` - The txid is not a valid hex string
+- `Decode` - Error during transaction decoding or serialization


NIT: I would like to see a note about the requirement of the wallet to track the transaction or we will not find it

moisesPompilio · 2026-06-22T22:02:42Z

b2e9a69 Applied the suggestions made by @jaoleal

…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.

Bump `corepc-types` to v0.14.0, bringing various updates and fixes to Bitcoin Core RPC type definitions and improving compatibility with newer RPC fields and responses.

…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 a helper for recursively comparing RPC responses while ignoring selected fields. Use it in the `getblockchaininfo`, `getblockheader`, `gettxout` and `getblock` tests to simplify response validation between Floresta and Bitcoin Core.

Merge `noraise_raw_request` and `noraise_raw_data_request` into a single `noraise_raw_request` helper. The new implementation automatically handles dictionary payloads using the previous JSON-RPC behavior and falls back to the raw data request path for non-dictionary payloads. Move commonly used RPC helper functions into `BaseRPC` so they are available to all nodes without requiring additional imports. Also remove redundant helper methods from the RPC base implementation.

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()`

moisesPompilio · 2026-06-26T11:25:49Z

eb13fe8 I rebased

jaoleal

ACK eb13fe8

Davidson-Souza

ACK eb13fe8

luisschwab

ACK eb13fe8

moisesPompilio force-pushed the get_raw_transaction branch from 044b3b7 to 6099cc1 Compare April 23, 2026 02:40

moisesPompilio self-assigned this Apr 23, 2026

moisesPompilio added the ecosystem Enable interoperability, compatibility and practical integration with the broader Bitcoin ecosystem label Apr 23, 2026

moisesPompilio added this to Floresta Apr 23, 2026

github-project-automation Bot moved this to Backlog in Floresta Apr 23, 2026

moisesPompilio moved this from Backlog to In progress in Floresta Apr 23, 2026

moisesPompilio mentioned this pull request Apr 23, 2026

[Enhancement] Add output script to descriptor converter (Bitcoin Core compliant) #987

Open

jaoleal self-requested a review April 23, 2026 10:46

moisesPompilio force-pushed the get_raw_transaction branch 3 times, most recently from 9547e4a to c9ac410 Compare May 15, 2026 01:55

moisesPompilio marked this pull request as ready for review May 15, 2026 01:59

moisesPompilio force-pushed the get_raw_transaction branch from c9ac410 to 22e29a2 Compare May 15, 2026 02:03

jaoleal moved this from In progress to Needs review in Floresta May 15, 2026

jaoleal reviewed May 15, 2026

View reviewed changes

csgui added this to the Q2/2026 milestone May 18, 2026

moisesPompilio force-pushed the get_raw_transaction branch from 22e29a2 to d67326e Compare May 19, 2026 18:34

Davidson-Souza requested changes May 20, 2026

View reviewed changes

moisesPompilio force-pushed the get_raw_transaction branch from d67326e to 7331f65 Compare May 21, 2026 15:57

jaoleal approved these changes May 21, 2026

View reviewed changes

jaoleal requested a review from Davidson-Souza May 25, 2026 14:02

moisesPompilio mentioned this pull request Jun 1, 2026

Electrum blockchain.transaction.get ignores the verbose argument (breaks Blue Wallet) #1107

Open

moisesPompilio force-pushed the get_raw_transaction branch from 7331f65 to d0b8750 Compare June 4, 2026 14:53

Davidson-Souza requested a review from jaoleal June 5, 2026 13:09

Davidson-Souza reviewed Jun 9, 2026

View reviewed changes

moisesPompilio force-pushed the get_raw_transaction branch 2 times, most recently from d4c3dc2 to b99a917 Compare June 17, 2026 22:19

csgui reviewed Jun 18, 2026

View reviewed changes

Comment thread crates/floresta-node/src/json_rpc/blockchain.rs Outdated

Comment thread crates/floresta-node/src/json_rpc/server.rs

moisesPompilio force-pushed the get_raw_transaction branch 2 times, most recently from a09ec26 to 20b0dad Compare June 18, 2026 19:19

moisesPompilio force-pushed the get_raw_transaction branch from 20b0dad to a0cf65d Compare June 22, 2026 11:46

jaoleal reviewed Jun 22, 2026

View reviewed changes

moisesPompilio force-pushed the get_raw_transaction branch 2 times, most recently from 0396af7 to b2e9a69 Compare June 22, 2026 21:50

moisesPompilio requested a review from jaoleal June 22, 2026 21:59

moisesPompilio added 7 commits June 25, 2026 22:37

chore(rpc): update corepc-types to v0.14.0

3623bc6

Bump `corepc-types` to v0.14.0, bringing various updates and fixes to Bitcoin Core RPC type definitions and improving compatibility with newer RPC fields and responses.

test(integration): add compare_fields helper

9760bbc

Add a helper for recursively comparing RPC responses while ignoring selected fields. Use it in the `getblockchaininfo`, `getblockheader`, `gettxout` and `getblock` tests to simplify response validation between Floresta and Bitcoin Core.

docs(rpc): add documentation for getrawtransaction RPC

eb13fe8

moisesPompilio force-pushed the get_raw_transaction branch from b2e9a69 to eb13fe8 Compare June 26, 2026 11:24

moisesPompilio requested review from Davidson-Souza and csgui June 26, 2026 11:24

jaoleal approved these changes Jun 30, 2026

View reviewed changes

moisesPompilio mentioned this pull request Jun 30, 2026

fix: inconsitencies on gettxoutproof #1113

Merged

Davidson-Souza approved these changes Jun 30, 2026

View reviewed changes

luisschwab approved these changes Jun 30, 2026

View reviewed changes

Davidson-Souza merged commit 73e748e into getfloresta:master Jun 30, 2026
14 checks passed

github-project-automation Bot moved this from Needs review to Done in Floresta Jun 30, 2026

	#[derive(Debug, Deserialize, Serialize)]
	#[derive(Debug, Deserialize, Serialize)]
	/// The information returned by a get_raw_tx

		raise TimeoutError(f"{error_msg} after {timeout} seconds")


		def compare_fields(candidate, reference, ignore_fields=None, float_tol=1e-8):

Uh oh!

Conversation

moisesPompilio commented Apr 23, 2026

Description and Notes

How to verify the changes you have done?

Uh oh!

moisesPompilio commented Apr 23, 2026

Uh oh!

moisesPompilio commented May 15, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Davidson-Souza commented May 20, 2026

Uh oh!

Davidson-Souza left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jaoleal left a comment

Choose a reason for hiding this comment

Uh oh!

Davidson-Souza commented Jun 2, 2026

Uh oh!

moisesPompilio commented Jun 4, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!