Support two sorobans for preflight

[graydon]

This is a companion patch to stellar/rs-soroban-env#1442 to support two sorobans in the RPC the same way that PR requires us to support two in stellar-core.

[dmkozh]

Thanks, this approach looks much better to me much better maintenance- and accuracy-wise (even if a bit hacky) - looking forward to rebase (and adding the missing file?).

[Shaptic]

I had a brief unresolved discussion with @leighmcculloch when implementing this feature (#249 (review)) that I think is worth resurfacing here:

When RPC has both the P22 and P23 host loaded for preflight (if I understand correctly, this means P23 is "compiled in" but is not being used until the upgrade kicks in), we want to make sure that we continue to return the JSON schema for P22 definitions. When the upgrade kicks in, we want to switch to P23 definitions (as well as P23 preflight itself). Does that mean we need the same adapter-esque code here to not always rely on curr when doing the XDR -> JSON translation?

Also, just to sanity check, we don't need to add support to the RPC (Go) side of preflight to perform this selection once its ingested the upgrade ledger, right? Since this is implied by the various let proto = ledger_info.protocol_version; lines in preflight.

But that does mean we need to perform the module swap on the RPC side for translation, since there's no such version check?

[Shaptic]

Separately, I think this should target our P22 branch v22-breaking-changes.

[leighmcculloch]

It's not obvious to me that the version of the XDR JSON schema should be tied to the protocol upgrade. At least we should consider the options.

When an upgrade occurs from v22 to v23, the change is to what features we will see show up in the XDR, such as a new enum variant becomes available for use.

What happens today (@Shaptic I realise you already know this, I'm just getting my thoughts out) is that the structure of the XDR / JSON changes out-of-band of protocol upgrades.

For example, when new XDR is released for an upcoming protocol, Stellar SDKs start adopting the new XDR structure before the protocol upgrade occurs.

Software first adopts the new XDR structure, and then new elements of that structure become in use after the protocol upgrade.

Continuing that existing pattern seems worth considering. i.e. To say that v23 of stellar-rpc uses the v23 XDR structure and therefore the structure of the output of v23 stellar-rpc is the same as v23 *-stellar-sdk.

I recognise that the JSON is the first time the structure, and not just the binary format, has to be coordinated, so what we are doing today with the structure really doesn't have to have any bearing on where we go from here.

But I think applications that need to update to the new XDR structure will need to do so before a protocol upgrade so there's one class of participants who will still want to have today's workflow.

If we were to do this, I think it would mean encoding the version of the stellar-rpc into a field of all responses, so that anyone parsing the responses could first check that metadata to confirm what version of rpc the response has been generated from. With any HTTP API it'd be reasonable, and preferred, to communicate the version separately in a header rather than in the response body, but I guess with JSON-RPC it will probably be in the body.

--

At some point @accordeiro suggested, could we make the JSON parsing backwards compatible, so that the Rust XDR Lib when the structure is updated we add in custom deserialisation logic that captures if the old format was in use. In theory this should be possible because the structural changes that typically take place are rather simple. Such as adding a new enum, or new union arm, or converting an optional into an enum or union. We'd need to allocate a small effort to spike out what's possible with the serde crate we use for JSON serialization as we have no experience with custom deserialization of this kind. (cc @janewang)

[Shaptic]

But I think applications that need to update to the new XDR structure will need to do so before a protocol upgrade so there's one class of participants who will still want to have today's workflow.

Is there a real expectation to support the schema at all from a programmatic perspective? There's no way today to turn that into a usable XDR structure, anyway. It's basically a visual convenience mechanism that devs shouldn't really rely on at all. I think we should have this discussion in parallel as we decide how this feature should be used and not let it block P22 work, so I'll be merging this 🫡

[graydon]

I've rebased this on the v22-breaking-changes branch and fixed as much of the CI issues as I can; it's currently failing on a mismatch with the stellar/go monorepo's XDR version, which I do not feel qualified to be adjusting (I don't even know what the policy is for doing that, much less how this relates to the v22-breaking-changes branch).

@Shaptic is this close enough that you can take it over the finish line now? I don't really understand what the conclusion of your conversation above is with @leighmcculloch -- if it's not quite right yet, I'm happy to make any further changes if you can be specific!

[Shaptic]

@graydon after changing the soroban-env revision in cfcf61b to stellar/rs-soroban-env@0497816 it looks like

meta::get_ledger_protocol_version(meta::INTERFACE_VERSION);

doesn't resolve anymore. Any ideas there?

[leighmcculloch]

@Shaptic the single value interface version was removed in p22, see:

• Update with stellar-xdr with split interface version rs-soroban-env#1453

[leighmcculloch]

meta::get_ledger_protocol_version(meta::INTERFACE_VERSION);

You should be able to do:

meta::INTERFACE_VERSION.protocol

[Shaptic]

Yep I found stellar/rs-soroban-env@df18803 after some digging, but

-pub(crate) const PROTOCOL: u32 = meta::get_ledger_protocol_version(meta::INTERFACE_VERSION);
+pub(crate) const PROTOCOL: u32 = meta::INTERFACE_VERSION.protocol;

results in a compiler error, which I assume is because the code needs to be different depending on if its prev or curr 😢

[leighmcculloch]

results in a compiler error, which I assume is because the code needs to be different depending on if its prev or curr 😢

It looks like this PR has the protocol version fixed at compile time and returning that protocol version in some places, but this PR is introducing the situation where either protocol can be active, so it seems like that code needs changing anyway. You should be able to call the appropriate existing or new logic depending on which is activated?

[Shaptic]

I think because the code was the same across both versions, it could safely live in the shared.rs file that selectively used the curr or next module to achieve the same goal across both, but now that there are different ways to retrieve it for each module, that code can't live here anymore.

I think I figured it out, though! Each method is used accordingly in 5e18dfd.