[FIX] [RPC] floresta-cli command `gettransaction` mismatch with florestad rpc server #616

TheMhv · 2025-08-16T20:11:44Z

What is the purpose of this pull request?

Which crates are being modified?

Description and Notes

#453
The command gettransaction is already implemented but is mismatch with command used inside florestad rpc server.

florestad expects getrawtransaction while floresta-cli sends gettransaction command.

I changed the command getrawtransaction to gettransaction that's a correct functionality inside florestad rpc server.

How to verify the changes you have done?

Verify gettransaction command work correctly using floresta-cli.

Contributor Checklist

I've followed the contribution guidelines
I've verified one of the following:
- Ran just pcc (recommended but slower)
- Ran just lint-features '-- -D warnings' && cargo test --release
- Confirmed CI passed on my fork
I've linked any related issue(s) in the sections above

Finally, you are encouraged to sign all your commits (it proves authorship and guards against tampering—see How (and why) to sign Git commits and GitHub's guide to signing commits).

TheMhv · 2025-08-18T14:21:23Z

You can see the gettransaction command is implemented inside floresta-cli:
https://github.com/vinteumorg/Floresta/blob/28b8d6fe03d64b990d77a73731c9d58bcd2ac017/crates/floresta-cli/src/rpc.rs#L272-L278
https://github.com/vinteumorg/Floresta/blob/28b8d6fe03d64b990d77a73731c9d58bcd2ac017/crates/floresta-cli/src/main.rs#L62-L64

And validate that florestad implemented this function:
https://github.com/vinteumorg/Floresta/blob/28b8d6fe03d64b990d77a73731c9d58bcd2ac017/florestad/src/json_rpc/server.rs#L108-L121

Davidson-Souza · 2025-08-18T14:48:24Z

You should actually rename to getrawtransaction in the CLI side. gettransaction and getrawtransaction are two different RPCs with different outputs. I think the one we have is closer to getrawtransaction

TheMhv · 2025-08-18T15:54:55Z

You should actually rename to getrawtransaction in the CLI side. gettransaction and getrawtransaction are two different RPCs with different outputs. I think the one we have is closer to getrawtransaction

Reading the Bitcoin RPC documentation, i found some mismatch with this implementations.

gettransaction is a command that returns information about in-wallet transactions, that's what function get_transaction inside florestad do, but this returns transaction in RawTxJson format, this format is from getrawtransaction command.

References:

Davidson-Souza · 2025-08-18T18:00:08Z

gettransaction is a command that returns information about in-wallet transactions, that's what function get_transaction inside florestad do, but this returns transaction in RawTxJson format, this format is from getrawtransaction command.

Yeah, I think it should be easy to implement gettransaction too, but the current implementation is for getrawtransaction

jaoleal

Thanks for delivering some work on this. Ive written some suggestions, care to take a look ?

doc/rpc/getrawtransaction.md

jaoleal · 2025-08-19T16:32:26Z

doc/rpc/getrawtransaction.md

+
+* `txid` - (string, required) The transaction id
+
+* `verbose` - (boolean, optional) If false, return a string, otherwise return a json object


Suggested change

* `verbose` - (boolean, optional) If false, return a string, otherwise return a json object

* `verbose` - (flag, optional) When set, returns more human-readable and detailed data from the specified transaction, otherwise it will just return the hex encoded transaction.

That's not a flag, it is a argument, defined in:

https://github.com/vinteumorg/Floresta/blob/87539ab542ceec84fd758c311b9a46a3a4a4b9d1/crates/floresta-cli/src/main.rs#L171

Oh... since we have only one level of verbosity can you make it a flag ?
The changes are always oriented as defined on the bitcoin core docs and is correct to use an Option but actually one of a u8 to theoretically cover its API, but i dont see an advantage in doing that so i think its better to correctly make it a flag, WYT ?

Having an Option of a Boolean doesnt make any sense XD

We use Option because we have a state when verbose is not defined, we will use as false statement. In other words, when verbose is set as None will be define as false.

https://github.com/vinteumorg/Floresta/blob/72f7549337aabd9a2633b60227e4603c426a8df0/crates/floresta-cli/src/rpc.rs#L273

Yes, please allow me to refrain better... The utility of an Option and a Boolean is similar when they represent two states, correct ? These two states that tell us what to do in such a case... when you have a boolean inside a Option<> you have a total of 3 states.

None
Some(true), and
Some(false)

but this is not necessary because you only need two states to make this verbose feature to work, that being a boolean alone representing whether the verbose is enabled right ?

The final result that i thought is a more concise and intuitive API delegating the presence check to the Clap crate because he alone can already tell which value to return in the case of the absence of an argument.

check this suggested change

We can apply this boolean statement into floresta-cli client, but this params need to be Option inside florestad rpc server, because other clients (like Bitcoin-cli) use this param as optional.

Hmmm, if youre trying to be strictly compliant to Bitcoin Core API the objective here is totally different.

in order to make it totally compatible you can follow something similar as i have done with the getblock command in this PR (the link will point to you the correct line)

so instead of having an Option<Bool> the change here needs to support an Option<u8>.

heres the documentation of the bitcoin core rpc at the latest version that im using to guide mine contributions to the #453

Also, to stick with the API design goal, care to change the inner method name ? get_transaction needs to be get_raw_transaction

doc/rpc/getrawtransaction.md

bin/floresta-cli/src/main.rs

jaoleal · 2025-08-19T16:50:47Z

crates/floresta-rpc/src/rpc.rs

@@ -43,7 +43,7 @@ pub trait FlorestaRPC {
    /// This method returns a transaction that's cached in our wallet. If the verbosity flag is
    /// set to false, the transaction is returned as a hexadecimal string. If the verbosity
    /// flag is set to true, the transaction is returned as a json object.
-    fn get_transaction(&self, tx_id: Txid, verbosity: Option<bool>) -> Result<Value>;
+    fn get_raw_transaction(&self, tx_id: Txid, verbosity: Option<bool>) -> Result<Value>;


here the docs needs to be changed to include the "getrawtransaction.md" one.

doc/rpc/getrawtransaction.md

jaoleal · 2025-08-20T15:48:58Z

bin/floresta-cli/src/main.rs

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


Suggested change

GetRawTransaction { txid: Txid, verbose: Option<bool> },

GetTransaction {

txid: Txid,

#[arg(

short = 'v',

long = "verbose",

required = false,

default_value_t = false

)]

verbose: bool,

},

jaoleal · 2025-08-20T15:53:28Z

Hi, @TheMhv some of the requested changes was marked as resolved but they werent addressed... Can you please take a look at these below ?

#616 (comment)
https://github.com/vinteumorg/Floresta/pull/616/files#r2285813409
https://github.com/vinteumorg/Floresta/pull/616/files#r2285817199

Please tell me if you need help addressing them or if you cant for some other reason.

doc/rpc/getrawtransaction.md

Davidson-Souza · 2025-11-27T13:24:59Z

Needs rebase

moisesPompilio · 2026-01-14T18:21:04Z

Hi @TheMhv , do you intend to continue working on this PR?

TheMhv · 2026-01-14T18:32:37Z

Hi @TheMhv , do you intend to continue working on this PR?

I'm little confused about the changes, I've need to review. Maybe you can help me to finish this PR

moisesPompilio · 2026-01-14T18:43:54Z

I'm little confused about the changes, I've need to review. Maybe you can help me to finish this PR

This RPC must be Bitcoin Core compliant. The verbosity parameter should be numeric, not boolean. Documentation needs to follow the project's template structure. RPC Reference: https://bitcoincore.org/en/doc/29.0.0/rpc/rawtransactions/getrawtransaction/

@jaoleal

…maded by @jaoleal

Davidson-Souza added bug Something isn't working RPC Changes something with our JSON-RPC interface labels Aug 18, 2025

Davidson-Souza added this to RPC Saga Aug 18, 2025

Davidson-Souza moved this to Outstanding Pull Requests in RPC Saga Aug 18, 2025

jaoleal reviewed Aug 19, 2025

View reviewed changes

TheMhv requested a review from jaoleal August 19, 2025 18:08

jaoleal reviewed Aug 20, 2025

View reviewed changes

doc/rpc/getrawtransaction.md Show resolved Hide resolved

doc/rpc/getrawtransaction.md Outdated Show resolved Hide resolved

TheMhv requested a review from jaoleal August 20, 2025 14:38

jaoleal suggested changes Aug 20, 2025

View reviewed changes

Davidson-Souza reviewed Aug 28, 2025

View reviewed changes

doc/rpc/getrawtransaction.md Outdated Show resolved Hide resolved

luisschwab assigned TheMhv Jan 28, 2026

TheMhv added 6 commits February 4, 2026 14:48

Fix: Rename rpc getrawtransaction command at floresta-cli crate

e0b0f31

Feat: Create getrawtransaction rpc doc

3d13ea9

Update getrawtransaction doc and fix rpc call function. Sugestions …

4438499

…maded by @jaoleal

Fix: Update getrawtransaction command documentation

6715394

Refact: Remove useless json types definition from getrawtransasction.md

a5687cb

Fix: Change json into a markdown list - getrawtransaction documentation

3a97f8f

TheMhv force-pushed the fix/mismatch_gettransaction branch from d5ad73d to 3a97f8f Compare February 4, 2026 17:50

TheMhv marked this pull request as draft February 4, 2026 18:09


		* `txid` - (string, required) The transaction id

		* `verbose` - (boolean, optional) If false, return a string, otherwise return a json object

	* `verbose` - (boolean, optional) If false, return a string, otherwise return a json object
	* `verbose` - (flag, optional) When set, returns more human-readable and detailed data from the specified transaction, otherwise it will just return the hex encoded transaction.

-    GetRawTransaction { txid: Txid, verbose: Option<bool> },
+        GetTransaction {
+        txid: Txid,
+        #[arg(
+            short = 'v',
+            long = "verbose",
+            required = false,
+            default_value_t = false
+        )]
+        verbose: bool,
+    },

[FIX] [RPC] floresta-cli command gettransaction mismatch with florestad rpc server #616

Are you sure you want to change the base?

[FIX] [RPC] floresta-cli command gettransaction mismatch with florestad rpc server #616

Uh oh!

Conversation

TheMhv commented Aug 16, 2025

What is the purpose of this pull request?

Which crates are being modified?

Description and Notes

How to verify the changes you have done?

Contributor Checklist

Uh oh!

TheMhv commented Aug 18, 2025

Uh oh!

Davidson-Souza commented Aug 18, 2025

Uh oh!

TheMhv commented Aug 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

References:

Uh oh!

Davidson-Souza commented Aug 18, 2025

Uh oh!

jaoleal left a comment

Choose a reason for hiding this comment

Uh oh!

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!

TheMhv Aug 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jaoleal commented Aug 20, 2025

Uh oh!

Uh oh!

Davidson-Souza commented Nov 27, 2025

Uh oh!

moisesPompilio commented Jan 14, 2026

Uh oh!

TheMhv commented Jan 14, 2026

Uh oh!

moisesPompilio commented Jan 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

[FIX] [RPC] floresta-cli command `gettransaction` mismatch with florestad rpc server #616

[FIX] [RPC] floresta-cli command `gettransaction` mismatch with florestad rpc server #616

TheMhv commented Aug 18, 2025 •

edited

Loading

TheMhv Aug 19, 2025 •

edited

Loading