stellar / rs-stellar-rpc-client

Rust Stellar RPC client.
0 stars 3 forks source link

feat!: Incorporate upstream breaking changes and update stellar deps #12

Open willemneal opened 2 weeks ago

willemneal commented 2 weeks ago

See breaking changes here: https://github.com/stellar/soroban-rpc/pull/280

Close https://github.com/stellar/rs-stellar-rpc-client/issues/16

2opremio commented 1 day ago

How is this coming along? We are about to release RPC rc1 and end-to-end tests are failing because of this (see https://github.com/stellar/soroban-rpc/actions/runs/11171362010/job/31055922343?pr=308 )

leighmcculloch commented 1 day ago

We are about to release RPC rc1 and end-to-end tests are failing because of this (see stellar/soroban-rpc/actions/runs/11171362010/job/31055922343?pr=308 )

The system tests won't pass until the CLI is updated I think? The CLI also runs system tests, so each time we break them it's like this chicken and egg problem.

classDiagram
    direction RL
    RPC <-- RPCClient
    RPCClient <-- CLI
    CLI <-- SystemTest
    SystemTest <-- RPC
    RPC <-- QuickStart
    QuickStart <-- SystemTest
    SystemTest <-- CLI
willemneal commented 1 day ago

@2opremio @leighmcculloch

Can we use the getNetwork endpoint to know the API? Then we could have and From trait to convert the responses.

And @2opremio besides the TransactionInfo flattening do the new types reflect the API. It seemed to me that there were more changes than the document @leighmcculloch is referencing in his issue.

leighmcculloch commented 1 day ago

Can we use the getNetwork endpoint to know the API?

That would mean calling getNetwork with every other call in a batch request... It seems like an uncommon pattern, but also I guess that would work.

Is there a reason we can't just make the responses decode both forms/structures at once?

willemneal commented 1 day ago

@leighmcculloch Looking into this, we can use #[serde(untaged)] with an enum that contains the differing fields. Then use #[serde(flatten)] in the top level struct. I will give this a try.

E.g.

use serde::{Deserialize, Serialize};

#[derive(Deserialize, Serialize, Debug)]
struct ResponseType {
    common_field: String,
    #[serde(flatten)]
    variable_field: DifferentResponses,
}

#[derive(Deserialize, Serialize, Debug)]
#[serde(untagged)]
enum DifferentResponse {
    OldResponse {
        old_field: String,
    },
    NewResponse {
        new_field: bool,
    },
}

#[serde(flatten)] will also fix the TransactionInfo issue.

leighmcculloch commented 23 hours ago

with an enum that contains the differing fields

Nice. Can we do one enum for each field? I think that's more true to how we want serde to fill and select fields.

leighmcculloch commented 23 hours ago

@willemneal For fields which are just a rename, we can use aliasing in serde, see: https://serde.rs/field-attrs.html

#[serde(alias = "old", alias = "new")]