`ServerInfo` response deserialized into incorrect result type due to incorrect attribute type

[denes710]

I attempted to use requests::ServerInfo to query the server state, but encountered an issue where the result field in the response was deserialized as results::Subscribe instead of results::ServerInfo.

This happens because the server_state_duration_us field in results::ServerInfo is currently typed as u64, while the Ripple documentation specifies it as a string:
link

Due to this type mismatch, Serde fails to match the response to results::ServerInfo, and instead falls back to the results::Subscribe variant, which contains only a phantom field and acting as a catch-all in this case.

I don’t have permission to push a new branch and open a pull request. It would be helpful if you could either grant the necessary permissions or apply the following diff to correct the issue:

diff --git a/src/models/results/server_info.rs b/src/models/results/server_info.rs
index a7b9450..e3214e6 100644
--- a/src/models/results/server_info.rs
+++ b/src/models/results/server_info.rs
@@ -60,7 +60,7 @@ pub struct Info<'a> {
     /// Current server state
     pub server_state: Cow<'a, str>,
     /// Microseconds in current state
-    pub server_state_duration_us: Option<u64>,
+    pub server_state_duration_us: Option<Cow<'a, str>>,
     /// Server state accounting information
     pub state_accounting: Option<Value>,
     /// Current UTC time according to server
@@ -176,7 +176,7 @@ mod tests {
                 ],
                 "pubkey_node": "n9KQK8yvTDcZdGyhu2EGdDnFPEBSsY5wEGpU5GgpygTgLFsjQyPt",
                 "server_state": "full",
-                "server_state_duration_us": 91758491912,
+                "server_state_duration_us": "91758491912",
                 "time": "2023-Sep-13 22:12:31.377492 UTC",
                 "uptime": 91948,
                 "validated_ledger": {
@@ -208,7 +208,7 @@ mod tests {
             "n9KQK8yvTDcZdGyhu2EGdDnFPEBSsY5wEGpU5GgpygTgLFsjQyPt"
         );
         assert_eq!(result.info.server_state, "full");
-        assert_eq!(result.info.server_state_duration_us, Some(91758491912));
+        assert_eq!(result.info.server_state_duration_us, Some("91758491912".into()));
         assert_eq!(
             result.info.time,
             Some("2023-Sep-13 22:12:31.377492 UTC".into())

[LimpidCrypto]

Do you maybe have the request you used, so I can reproduce?

[denes710]

@LimpidCrypto

use xrpl::models::results::server_info::ServerInfo as ServerInfoResult;
use xrpl::models::requests::server_info::ServerInfo;
use xrpl::asynch::clients::{AsyncJsonRpcClient, XRPLAsyncClient};

let client = AsyncJsonRpcClient::connect(api_url.parse().unwrap());
let response = client.request(ServerInfo::new(Some(Cow::Borrowed("my_id"))).into()).await.unwrap();
// let server_info: ServerInfoResult = response.try_into()?; // not working, because response.result is results::Subscribe

[denes710]

Hi there! @LimpidCrypto

I have just noticed that "network_id" member is missing from the Info struct in results::server_info::ServerInfo. Could you also correct this issue?

This attribute is placed in the response object here and we also use this value from the response in one of our functions.

[LimpidCrypto]

PRs:
#113
#114

Thank you for contributing :)

[PumpkinSeed]

Hey @LimpidCrypto,

I'm the one who took over @denes710 task. I actually came with a few other similar errors. I'm afraid that the impl_from_result macro is pretty error prone. I bring you a few more examples. We tested against ripple 2.4.0. I also wrote a proxy between the rust client and the ripple node, so I could log out the req/resp.

---

1. XRPL Model error: Unexpected result type (expected "AccountInfo", got "LedgerCurrent")

Request:

{"method":"account_info","params":[{"account":"rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh","ledger_index":"current"}]}

Response:

{"result":{"account_data":{"Account":"rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh","Balance":"100000000000000000","Flags":0,"LedgerEntryType":"AccountRoot","OwnerCount":0,"PreviousTxnID":"0000000000000000000000000000000000000000000000000000000000000000","PreviousTxnLgrSeq":0,"Sequence":1,"index":"2B6AC232AA4C4BE41BF49D2459FA4A0347E1B543A4C92FCEE0821C0201E2E9A8"},"account_flags":{"defaultRipple":false,"depositAuth":false,"disableMasterKey":false,"disallowIncomingXRP":false,"globalFreeze":false,"noFreeze":false,"passwordSpent":false,"requireAuthorization":false,"requireDestinationTag":false},"ledger_current_index":3,"status":"success","validated":false}}

That was a part of an autofill function call:

let mut payment_tx = transactions::payment::Payment{
            common_fields: transactions::CommonFields {
                account: Cow::from(wallet.classic_address.clone()),
                transaction_type: transactions::TransactionType::Payment,
                fee: Some(models::XRPAmount::from("10000")),
                // Striped out the unnecessary part
            },

            amount: models::Amount::from(models::XRPAmount::from("1000000000")),
            destination: Cow::from("rfvtbrXSxLsxVWDktR4sdzjJgv8EnMKFKG"),
            // Striped out the unnecessary part
        };

        let client = clients::AsyncJsonRpcClient::connect("http://localhost:55512".parse().unwrap());

        match transaction::autofill(&mut payment_tx, &client, Some(1)).await {
            Ok(_) => println!("Transaction autofilled successfully"),
            Err(e) => println!("Error autofilling transaction: {}", e),
}

---

2. XRPL Model error: Unexpected result type (expected "Submit", got "Subscribe")

Request:

{"method":"submit","params":[{"tx_blob":"120000220000000061400000003B9ACA0068400000000000271073210330E7FC9D56BB25D6893BA3F317AE5BCF33B3291BD63DB32654A313222F7FD02074473045022100E97EBA30FDCB27489745E83900717FAE798DC491271874531D4688BC7AD39E5A022020DA0FDAFA4874BD44B259BE211FBDA6065EB916B6CBC9A5C07B6AAECBB0736F8114B5F762798A53D543A014CAF8B297CFF8F2F937E883144C01DCE0C4B07277547BB8E83BEB2E1842E8E2D6"}]}

Response:

{"result":{"error":"invalidTransaction","error_exception":"Field 'Sequence' is required but missing.","request":{"command":"submit","tx_blob":"120000220000000061400000003B9ACA0068400000000000271073210330E7FC9D56BB25D6893BA3F317AE5BCF33B3291BD63DB32654A313222F7FD02074473045022100E97EBA30FDCB27489745E83900717FAE798DC491271874531D4688BC7AD39E5A022020DA0FDAFA4874BD44B259BE211FBDA6065EB916B6CBC9A5C07B6AAECBB0736F8114B5F762798A53D543A014CAF8B297CFF8F2F937E883144C01DCE0C4B07277547BB8E83BEB2E1842E8E2D6"},"status":"error"}}

That make sense, because it's an error, but the result what I got back isn't described it.

Code:

// Followup from above.
match transaction::sign(&mut payment_tx, &wallet, false) {
            Ok(_) => println!("Transaction signed successfully"),
            Err(e) => println!("Error signing transaction: {}", e),
        };

        println!("Transaction: {:?}", payment_tx);

        match transaction::submit(&payment_tx, &client).await {
            Ok(response) => println!("Transaction submitted successfully: {:?}", response),
            Err(e) => println!("Error submitting transaction: {}", e),
        };

[PumpkinSeed]

One of my proposal for the parser, instead of using Rust's pattern matching in a macro, what if there is a transaction_type-based parser?

Since the response will always respect the request's transaction type, the actual transactions::TransactionType::X can implement parser. For example in case of transactions::TransactionType::Payment it can call transactions::TransactionType::Payment::parse(resp) and it will return the corresponding type.

Let me know what you think.

[PumpkinSeed]

Based on my following investigation, I found that the impl_from_result macro is error-prone, because it's not capable of properly determine the actual type of the result. In case of a AsyncJsonRpcClient.request_impl, it determines the result's type incorrectly.

I'm still investigating the best solution for the issue.

[LimpidCrypto]

Maybe I'm not getting it right. The 1 example you've provided deserializes correctly. The Issue with the second is example is that it is an error. I think we still have the old rippled error response model. We would need to update it to be compatible with rippled v2. We can use the xrpl-py Response model as reference. We should also make the Subscribe result model more concrete for the different stream types so it isn't a fallback for all responses that can't be deserialized.

For the Submit result we currently deserialize the tx_json to serde_json::Value as we weren't sure if the returned transaction json have same fields than the models::transactions have.

[LimpidCrypto]

For me this would mean we just need more testing so we can have a more robust deserializing

[LimpidCrypto]

I'm sorry I tested yesterday and remembered it wrong. The first example also did not deserialize correctly. It was due to missing #[serde(default)] for the AccountFlags

[PumpkinSeed]

@LimpidCrypto You are right, that the Submit is not a serialization issue, because of the missing Sequence what the autofill didn't fill since the first deserialize error.

In the first case, the ledger_current_index field contained by both LedgerCurrent and AccountInfo. In case of the sequence getter, that is the only one field, so not a chance that a pattern match can identify it properly.

This is what it looks like in the debugger: