Draft of state archival simulation support #1475
Conversation
| ext: if let Some(proof) = new_entries_proof { | ||
| SorobanTransactionDataExt::V1(vec![proof].try_into().unwrap()) | ||
| } else { | ||
| SorobanTransactionDataExt::V0 |
There was a problem hiding this comment.
This is fine for now, but in my prototype I've had all SorobanTransactionDataExt default to V1 regardless of if there's a proof or not in p23 (it's just an empty vector if no proof is required). I don't think this is strictly necessary and it does increase TX size overhead slightly, but I thought the status quo for XDR changes like this is to require the new version of the extension. I don't feel strongly about this either way, just wanted to flag it.
There was a problem hiding this comment.
Hmm actually this is not currently compatible with my prototype, where a soroban tx gets dropped and is not considered valid if we're in p23 and SorobanTransactionDataExt is not v1. I can either change my prototype or we can change it here.
There was a problem hiding this comment.
This is not up to core to decide, is it? I think we should keep V0 around for the sake of backwards compatibility. I think for tx envelope changes we should be as backwards compatible as possible and shouldn't deprecate any exts unless they're somehow broken/incompatible with the protocol (which isn't the case here).
| } else { | ||
| Ok(None) | ||
| if matches!(entry_state.state, LedgerEntryArchivalState::Archived(_)) { |
There was a problem hiding this comment.
Nit: This invariant can be strengthened to if !matches!(entry_state.state, LedgerEntryArchivalState::New(_))
SirTyson
force-pushed
the
archival_simulation
branch
from
236f87f to
83f363c
Compare
No description provided.