Skip to content

improvement(RPC): unified interface for legacy and current RPC interfaces #2450

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

Merged
onur-ozkan merged 7 commits into from
Jun 9, 2025
Merged

improvement(RPC): unified interface for legacy and current RPC interfaces #2450

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
05544a7
re-write legacy `process_single_request`
onur-ozkan May 8, 2025
5dc1cd8
provide unified RPC interface for clients
onur-ozkan May 8, 2025
b2e7184
remove `mmrpc` from various tests to have a coverage on recent change
onur-ozkan May 8, 2025
52b6889
use `ERR` macro instead of `ERRL`
onur-ozkan May 8, 2025
1083d11
remove unknown method test for legacy dispatcher
onur-ozkan May 8, 2025
7523eb2
add info log
onur-ozkan May 26, 2025
da1d183
nit
onur-ozkan May 27, 2025
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
21 changes: 17 additions & 4 deletions mm2src/mm2_main/src/rpc.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,12 +175,25 @@ fn response_from_dispatcher_error(
response.serialize_http_response()
}

async fn process_single_request(ctx: MmArc, req: Json, client: SocketAddr) -> Result<Response<Vec<u8>>, String> {
async fn process_single_request(ctx: MmArc, mut req: Json, client: SocketAddr) -> Result<Response<Vec<u8>>, String> {
let local_only = ctx.conf["rpc_local_only"].as_bool().unwrap_or(true);
if req["mmrpc"].is_null() {
return dispatcher_legacy::process_single_request(ctx, req, client, local_only)
.await
.map_err(|e| ERRL!("{}", e));
match dispatcher_legacy::process_single_request(ctx.clone(), req.clone(), client, local_only).await {
Copy link
Copy Markdown
Collaborator

@dimxy dimxy May 28, 2025

Choose a reason for hiding this comment

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

Should we not first try rpc version 2 rather than legacy?
to make preference for v2, in case of identical rpc name for v2 and legacy

Copy link
Copy Markdown
Author

@onur-ozkan onur-ozkan May 29, 2025

Choose a reason for hiding this comment

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

This approach will cause BC on identical RPC names as they don't set mmrpc field.

Ok(t) => return Ok(t),

Err(dispatcher_legacy::LegacyRequestProcessError::NoMatch) => {
// Try the v2 implementation
req["mmrpc"] = json!("2.0");
info!(
"Couldn't resolve '{}' RPC using the legacy API, trying v2 (mmrpc: 2.0) instead.",
req["method"]
);
},

Err(e) => {
return ERR!("{}", e);
},
};
}

let id = req["id"].as_u64().map(|id| id as usize);
Expand Down
34 changes: 28 additions & 6 deletions mm2src/mm2_main/src/rpc/dispatcher/dispatcher_legacy.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,27 +117,49 @@ pub fn dispatcher(req: Json, ctx: MmArc) -> DispatcherRes {
})
}

#[derive(Debug, Display)]
pub enum LegacyRequestProcessError {
#[display(fmt = "Selected method is not allowed: {reason}")]
NotAllowed { reason: String },
#[display(fmt = "No such method")]
NoMatch,
#[display(fmt = "RPC call failed: {reason}")]
Failed { reason: String },
}

pub async fn process_single_request(
ctx: MmArc,
req: Json,
client: SocketAddr,
local_only: bool,
) -> Result<Response<Vec<u8>>, String> {
) -> Result<Response<Vec<u8>>, LegacyRequestProcessError> {
// https://github.com/artemii235/SuperNET/issues/368
if local_only && !client.ip().is_loopback() && !PUBLIC_METHODS.contains(&req["method"].as_str()) {
return ERR!("Selected method can be called from localhost only!");
return Err(LegacyRequestProcessError::NotAllowed {
reason: "Selected method can only be called from localhost.".to_owned(),
});
}
let rate_limit_ctx = RateLimitContext::from_ctx(&ctx).unwrap();
if rate_limit_ctx.is_banned(client.ip()).await {
return ERR!("Your ip is banned.");
return Err(LegacyRequestProcessError::NotAllowed {
reason: "Your IP is banned.".to_owned(),
});
}
try_s!(auth(&req, &ctx, &client).await);
auth(&req, &ctx, &client)
.await
.map_err(|reason| LegacyRequestProcessError::Failed { reason })?;

let handler = match dispatcher(req, ctx.clone()) {
DispatcherRes::Match(handler) => handler,
DispatcherRes::NoMatch(_) => return ERR!("No such method."),
DispatcherRes::NoMatch(_) => {
return Err(LegacyRequestProcessError::NoMatch);
},
};
Ok(try_s!(handler.compat().await))

handler
.compat()
.await
.map_err(|reason| LegacyRequestProcessError::Failed { reason })
}

/// The set of functions that convert the result of the updated handlers into the legacy format.
Expand Down
10 changes: 0 additions & 10 deletions mm2src/mm2_main/tests/mm2_tests/lightning_tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,6 @@ async fn open_channel(
let request = mm
.rpc(&json!({
"userpass": mm.userpass,
"mmrpc": "2.0",
"method": "lightning::channels::open_channel",
"params": {
"coin": coin,
Expand Down Expand Up @@ -197,7 +196,6 @@ async fn close_channel(mm: &MarketMakerIt, uuid: &str, force_close: bool) -> Jso
let request = mm
.rpc(&json!({
"userpass": mm.userpass,
"mmrpc": "2.0",
"method": "lightning::channels::close_channel",
"params": {
"coin": "tBTC-TEST-lightning",
Expand All @@ -221,7 +219,6 @@ async fn add_trusted_node(mm: &MarketMakerIt, node_id: &str) -> Json {
let request = mm
.rpc(&json!({
"userpass": mm.userpass,
"mmrpc": "2.0",
"method": "lightning::nodes::add_trusted_node",
"params": {
"coin": "tBTC-TEST-lightning",
Expand All @@ -243,7 +240,6 @@ async fn generate_invoice(mm: &MarketMakerIt, amount_in_msat: u64) -> Json {
let request = mm
.rpc(&json!({
"userpass": mm.userpass,
"mmrpc": "2.0",
"method": "lightning::payments::generate_invoice",
"params": {
"coin": "tBTC-TEST-lightning",
Expand All @@ -267,7 +263,6 @@ async fn pay_invoice(mm: &MarketMakerIt, invoice: &str) -> Json {
let request = mm
.rpc(&json!({
"userpass": mm.userpass,
"mmrpc": "2.0",
"method": "lightning::payments::send_payment",
"params": {
"coin": "tBTC-TEST-lightning",
Expand All @@ -293,7 +288,6 @@ async fn get_payment_details(mm: &MarketMakerIt, payment_hash: &str) -> Json {
let request = mm
.rpc(&json!({
"userpass": mm.userpass,
"mmrpc": "2.0",
"method": "lightning::payments::get_payment_details",
"params": {
"coin": "tBTC-TEST-lightning",
Expand Down Expand Up @@ -428,7 +422,6 @@ fn test_connect_to_node() {

let connect = block_on(mm_node_2.rpc(&json!({
"userpass": mm_node_2.userpass,
"mmrpc": "2.0",
"method": "lightning::nodes::connect_to_node",
"params": {
"coin": "tBTC-TEST-lightning",
Expand Down Expand Up @@ -468,7 +461,6 @@ fn test_open_channel() {

let list_channels_node_1 = block_on(mm_node_1.rpc(&json!({
"userpass": mm_node_1.userpass,
"mmrpc": "2.0",
"method": "lightning::channels::list_open_channels_by_filter",
"params": {
"coin": "tBTC-TEST-lightning",
Expand Down Expand Up @@ -497,7 +489,6 @@ fn test_open_channel() {

let list_channels_node_2 = block_on(mm_node_2.rpc(&json!({
"userpass": mm_node_2.userpass,
"mmrpc": "2.0",
"method": "lightning::channels::list_open_channels_by_filter",
"params": {
"coin": "tBTC-TEST-lightning",
Expand Down Expand Up @@ -547,7 +538,6 @@ fn test_send_payment() {

let send_payment = block_on(mm_node_2.rpc(&json!({
"userpass": mm_node_2.userpass,
"mmrpc": "2.0",
"method": "lightning::payments::send_payment",
"params": {
"coin": "tBTC-TEST-lightning",
Expand Down
10 changes: 0 additions & 10 deletions mm2src/mm2_main/tests/mm2_tests/mm2_tests_inner.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,16 +53,6 @@ cfg_wasm32! {
fn test_rpc() {
let (_, mm, _dump_log, _dump_dashboard) = mm_spat();

let no_method = block_on(mm.rpc(&json! ({
"userpass": mm.userpass,
"coin": "RICK",
"ipaddr": "electrum1.cipig.net",
"port": 10017
})))
.unwrap();
assert!(no_method.0.is_server_error());
assert_eq!((no_method.2)[ACCESS_CONTROL_ALLOW_ORIGIN], "http://localhost:4000");
Comment on lines -56 to -64
Copy link
Copy Markdown
Author

@onur-ozkan onur-ozkan May 8, 2025

Choose a reason for hiding this comment

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

This was testing unknown methods on the legacy dispatcher (not very useful to cover to be honest). The behavior for unknown RPCs in the legacy dispatcher has changed: if a method isn't found there, we now try to find it in the current dispatcher.


let not_json = mm.rpc_str("It's just a string").unwrap();
assert!(not_json.0.is_server_error());
assert_eq!((not_json.2)[ACCESS_CONTROL_ALLOW_ORIGIN], "http://localhost:4000");
Expand Down
Loading