Skip to content

Centralize server-side defaults and align addnode behavior with Bitcoin Core #986

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
moisesPompilio wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Centralize server-side defaults and align addnode behavior with Bitcoin Core #986

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6c1cacd
fix(node): make v2transport optional in addnode, allowing server-side…
moisesPompilio Apr 15, 2026
b17939b
chore(rpc): remove client-side defaults via RpcArg parameter abstraction
moisesPompilio Apr 15, 2026
e077196
test(rpc): add comprehensive unit tests for all RPC methods
moisesPompilio Apr 24, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 3 additions & 12 deletions bin/floresta-cli/src/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,10 +104,7 @@ fn do_request(cmd: &Cli, client: Client) -> anyhow::Result<String> {
node,
command,
v2transport,
} => {
let transport = v2transport.unwrap_or(false);
serde_json::to_string_pretty(&client.add_node(node, command, transport)?)?
}
} => serde_json::to_string_pretty(&client.add_node(node, command, v2transport)?)?,
Methods::DisconnectNode {
node_address,
node_id,
Expand All @@ -117,14 +114,8 @@ fn do_request(cmd: &Cli, client: Client) -> anyhow::Result<String> {
vout,
script,
height_hint,
} => serde_json::to_string_pretty(&client.find_tx_out(
txid,
vout,
script,
height_hint.unwrap_or(0),
)?)?,
} => serde_json::to_string_pretty(&client.find_tx_out(txid, vout, script, height_hint)?)?,
Methods::GetMemoryInfo { mode } => {
let mode = mode.unwrap_or("stats".to_string());
serde_json::to_string_pretty(&client.get_memory_info(mode)?)?
}
Methods::GetRpcInfo => serde_json::to_string_pretty(&client.get_rpc_info()?)?,
Expand Down Expand Up @@ -340,7 +331,7 @@ pub enum Methods {
)]
DisconnectNode {
node_address: String,
node_id: Option<usize>,
node_id: Option<u32>,
},

#[command(name = "findtxout")]
Expand Down
1 change: 1 addition & 0 deletions crates/floresta-node/src/florestad.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -478,6 +478,7 @@ impl Florestad {
.map(|x| Self::resolve_hostname(x, 8332))
.transpose()?,
format!("{data_dir}/debug.log"),
!self.config.allow_v1_fallback,
));

if self.json_rpc.set(server).is_err() {
Expand Down
4 changes: 3 additions & 1 deletion crates/floresta-node/src/json_rpc/network.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ impl<Blockchain: RpcChain> RpcImpl<Blockchain> {
&self,
node_address: String,
command: String,
v2transport: bool,
v2transport: Option<bool>,
Copy link
Copy Markdown
Contributor

@Micah-Shallom Micah-Shallom Apr 23, 2026

Choose a reason for hiding this comment

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

is this change worth documenting as a minor bump for downstream users of this RPC? or maybe that doesnt matter much right now since the commmunity building on floresta is not large yet?

more of a check question for myself too since i have recently worked on an RPC that modified fieldnames

Copy link
Copy Markdown
Collaborator Author

@moisesPompilio moisesPompilio Apr 24, 2026

Choose a reason for hiding this comment

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

This is already documented in the RPC docs; this field is optional, so nothing changes for the user.

Copy link
Copy Markdown
Member

@jaoleal jaoleal Apr 24, 2026

Choose a reason for hiding this comment

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

I think its a fair point, i think its fine to break things now because we didnt made the 1.0 release yet. The user count is not something we can be much sure of, AFAIK...

) -> Result<Value> {
// Try to parse both IP address and port.
let (addr, port) = if let Ok(socket_addr) = node_address.parse::<SocketAddr>() {
Expand All @@ -52,6 +52,8 @@ impl<Blockchain: RpcChain> RpcImpl<Blockchain> {
(ip, default_port)
};

let v2transport = v2transport.unwrap_or(self.default_connection_is_v2);

let _ = match command.as_str() {
"add" => self.node.add_peer(addr, port, v2transport).await,
"remove" => self.node.remove_peer(addr, port).await,
Expand Down
8 changes: 5 additions & 3 deletions crates/floresta-node/src/json_rpc/server.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ pub struct RpcImpl<Blockchain: RpcChain> {
pub(super) inflight: Arc<RwLock<HashMap<Value, InflightRpc>>>,
pub(super) log_path: String,
pub(super) start_time: Instant,
pub(super) default_connection_is_v2: bool,
}

type Result<T> = std::result::Result<T, JsonRpcError>;
Expand Down Expand Up @@ -320,7 +321,7 @@ async fn handle_json_rpc_request(
let vout = get_numeric(&params, 1, "vout")?;
let script = get_string(&params, 2, "script")?;
let script = ScriptBuf::from_hex(&script).map_err(|_| JsonRpcError::InvalidScript)?;
let height = get_numeric(&params, 3, "height")?;
let height = get_optional_field(&params, 3, "height", get_numeric)?.unwrap_or(0);

let state = state.clone();
state.find_tx_out(txid, vout, script, height).await
Expand Down Expand Up @@ -364,8 +365,7 @@ async fn handle_json_rpc_request(
"addnode" => {
let node = get_string(&params, 0, "node")?;
let command = get_string(&params, 1, "command")?;
let v2transport =
get_optional_field(&params, 2, "V2transport", get_bool)?.unwrap_or(false);
let v2transport = get_optional_field(&params, 2, "V2transport", get_bool)?;

state
.add_node(node, command, v2transport)
Expand Down Expand Up @@ -749,6 +749,7 @@ impl<Blockchain: RpcChain> RpcImpl<Blockchain> {
block_filter_storage: Option<Arc<NetworkFilters<FlatFiltersStore>>>,
address: Option<SocketAddr>,
log_path: String,
default_connection_is_v2: bool,
) {
let address = address.unwrap_or_else(|| {
format!("127.0.0.1:{}", Self::get_port(&network))
Expand Down Expand Up @@ -789,6 +790,7 @@ impl<Blockchain: RpcChain> RpcImpl<Blockchain> {
inflight: Arc::new(RwLock::new(HashMap::new())),
log_path,
start_time: Instant::now(),
default_connection_is_v2,
}));

axum::serve(listener, router)
Expand Down
Loading
Loading