Skip to content

[REFACTOR][RPC] Centralize RPC traits and method names#1067

Open
moisesPompilio wants to merge 7 commits into
getfloresta:masterfrom
moisesPompilio:centralize-trait-and-type-rpc
Open

[REFACTOR][RPC] Centralize RPC traits and method names#1067
moisesPompilio wants to merge 7 commits into
getfloresta:masterfrom
moisesPompilio:centralize-trait-and-type-rpc

Conversation

@moisesPompilio
Copy link
Copy Markdown
Collaborator

Description and Notes

The RPC trait and method definitions were centralized so that floresta-rpc and floresta-node stay aligned and compatible.

At the moment, floresta-rpc implements RPC methods that are consumed by the node, while floresta-node exposes the server-side implementation that serves the same methods. To avoid inconsistencies between both crates, this change introduces shared RPC traits that define the contract for these methods in a single place.

The main challenge is that floresta-rpc needs synchronous traits, while floresta-node needs asynchronous ones. To solve this, this change introduces the maybe-async macro, which allows the same trait definitions to support both sync and async implementations through a single code path.

In addition to the trait unification, the RPC macros were reorganized into categories similar to Bitcoin Core:

  • wallet
  • blockchain
  • control
  • network
  • util
  • rawtransactions

It also introduces RpcMethods, an enum that acts as the single source of truth for all RPC method names. It provides to_string(), as_str(), and FromStr implementations so both floresta-rpc and floresta-node can use the same method definitions safely. This prevents spelling mismatches and avoids duplicating method names across the codebase.

How to verify the changes you have done?

  • Build floresta-rpc and floresta-node to confirm the shared RPC traits compile correctly in both sync and async contexts.
  • Verify that RpcMethods can be converted with to_string(), as_str(), and FromStr.
  • Check that RPC method names are now defined in a single place and reused consistently across the codebase.
  • Confirm that the RPC trait categories are correctly split into:
    • wallet
    • blockchain
    • control
    • network
    • util
    • rawtransactions
  • Ensure the workspace builds correctly after the maybe-async integration and the crate-by-crate build adjustment.

@moisesPompilio moisesPompilio force-pushed the centralize-trait-and-type-rpc branch from ed4a6ff to 2443380 Compare May 15, 2026 21:43
@luisschwab luisschwab added code quality Generally improves code readability and maintainability RPC Changes something with our JSON-RPC interface labels May 15, 2026
@github-project-automation github-project-automation Bot moved this to Backlog in Floresta May 15, 2026
@moisesPompilio moisesPompilio added the reliability Related to runtime reliability, stability and production readiness label May 15, 2026
@moisesPompilio moisesPompilio added this to the Q2/2026 milestone May 15, 2026
@moisesPompilio moisesPompilio moved this from Backlog to Needs review in Floresta May 15, 2026
Replace hardcoded method name strings with a single source of truth.
The RpcMethods enum provides FromStr, to_string(), and as_str() for
type-safe method name handling, preventing divergence across codebase.
Split RPC methods into category-specific traits (Blockchain, Wallet,
Network, RawTransaction, Control) for better code organization,
segregation, and flexible implementation across RPC and CLI.
@moisesPompilio moisesPompilio force-pushed the centralize-trait-and-type-rpc branch from 2443380 to c357d5a Compare May 21, 2026 19:48
Introduce maybe_async procedural macro to NetworkRpc trait to support
both async and sync implementations. This allows:

- Shared trait definitions between floresta-node (async) and floresta-rpc (sync)
- Consistent interface across different implementations
- NetworkRpc trait now implemented in floresta-node to ensure network
  operations are compatible between floresta-node and floresta-rpc

Fix build compatibility issue:
- maybe_async feature conflicts when building entire workspace
- Adjusted justfile to compile individual crates by path instead of
  workspace super-set to prevent async feature leaking into floresta-cli
- floresta-cli now builds without async feature to preserve RPC implementation
Add maybe_async support to WalletRpc trait enabling async/sync flexibility.
WalletRpc now implemented in floresta-node to ensure consistency.
…a-node

Enable async/sync flexibility for ControlRpc trait with maybe_async.
Provides unified interface for control operations (get_memory_info,
get_rpc_info, stop, uptime) across floresta-node and floresta-rpc.
…esta-node

Enable async/sync flexibility for BlockchainRpc trait with maybe_async.
Provides unified interface for blockchain operations across floresta-node
and floresta-rpc, completing the RPC trait migration pattern.
Enable async/sync flexibility for RawTransactionRpc trait with maybe_async.
Remove corepc_types dependency from floresta-node, centralizing all RPC
types in floresta-rpc as single source of truth.
@moisesPompilio moisesPompilio force-pushed the centralize-trait-and-type-rpc branch from c357d5a to 167b5c2 Compare May 21, 2026 19:53
@moisesPompilio
Copy link
Copy Markdown
Collaborator Author

167b5c2 I rebased

self.call("loaddescriptor", &[Value::String(descriptor)])
}

fn get_block_filter(&self, height: u32) -> Result<String> {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You removed get_block_filter?

@@ -0,0 +1,156 @@
use core::fmt::Debug;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SPDX headers and module-level docs

use core::fmt::Debug;
use std::str::FromStr;

macro_rules! define_rpc_methods {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Docs for this macro would be nice!

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lorenzolfm don't look over here.

Methods::GetBlockCount => serde_json::to_string_pretty(&client.get_block_count()?)?,
Methods::GetTxOut { txid, vout } => {
serde_json::to_string_pretty(&client.get_tx_out(txid, vout)?)?
serde_json::to_string_pretty(&client.get_tx_out(txid, vout, false)?)?
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's this false for?

/// Returns the transaction, assuming it is cached by our watch only wallet
#[command(name = "gettransaction")]
GetTransaction { txid: Txid, verbose: Option<bool> },
GetTransaction { txid: Txid, verbose: Option<u32> },
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

verbosity then

script: String,
height_hint: u32,
) -> Result<Value> {
height: u32,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer height_hint

Comment on lines +98 to +100
if result.is_null() {
return Err(Error::TxOutNotFound);
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this right? Shouldn't this be handled at server side?

Comment on lines +48 to +55
#!/usr/bin/env bash
set -e
CRATES=$(cargo metadata --format-version 1 --no-deps | jq -r '.packages[] | select(.source == null and (.targets | map(.kind | contains(["lib"])) | any)) | .name' | tr '\n' ' ')
echo "Building docs for crates: $CRATES"
for crate in $CRATES; do
echo "Running cargo doc for: $crate"
RUSTDOCFLAGS="--cfg docsrs -D warnings" cargo +nightly doc -p "$crate" --no-deps --lib --all-features --document-private-items || exit 1
done
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, we need to make sure we can build docs for https://docs.gefloresta.sh

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

code quality Generally improves code readability and maintainability reliability Related to runtime reliability, stability and production readiness RPC Changes something with our JSON-RPC interface

Projects

Status: Needs review

Development

Successfully merging this pull request may close these issues.

3 participants