SEP-11 (txrep) support for protocol 13 changes #618
Labels
Comments
|
Further to point 1: txRep exposes the structure of the transaction. With the definition of a v0 transactions changing in protocol 13, how should parsers handle |
|
We ideally wouldn't have to type |
|
If we allow the tags to be omitted do we assume backwards consistency and default to v0 or forward consistency and use the latest version? If we go with the former then txrep and XDR encoding to and from will be consistent across XDR changes, but also we'll be building old transaction versions for new txreps. If we go with the latter then a txrep written a year ago will generate different XDR with a new version won't it? In principle should a Txrep I write today encode to the exact same XDR one year from now, or should it encode only to a conceptually consistent transaction that may not be byte for byte equality? My expectation would be that txrep is only for temporary representation of a transaction and long term storage should be in XDR format, which would mean going with the latter and assuming the latest version. |
|
I'm with @leighmcculloch -- I feel that txrep should represent the meaning behind the transaction, not necessarily the exact bytes of the XDR. Taking that stance however means that any signatures stored in the txrep might be worthless. |
|
I'd go as far as saying that any implementation of txrep should only guarantee that an XDR message decoded into txrep must re-encode back into the same XDR message inside a single execution of the program, but outside of a single execution of a program that can't be guaranteed. So short term should be short term in memory. If there are uses beyond that maybe we could specify a way to generate verbose txrep that included the tag and would be stable? |
|
So to be clear, I'm not advocating introducing any ambiguities. The discriminant would still be there, but the branch name would be omitted. This doesn't capture all of XDR in the event that you have nested unions with the same discriminant name. But we could say it only works for non-unions inside unions. More concretely, a transaction in txrep now looks like this, where But what we choose affects, for instance, how feebump transactions are represented. |
|
Hi @pselden, the latest version of the Stellar transaction compiler stc supports fee bump transactions and muxed accounts. The flutter sdk also supports them. I think that this issue can be closed. This is the txrep generated by stc for a fee bump transaction having a muxed account as fee source: XDR: |
|
This issue is stale because it has been open for 30 days with no activity. It will be closed in 5 days unless the stale label is removed, or a comment is posted. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
When testing and adding support for protocol 13 in https://github.com/stellarguard/txrep, I realized that there's currently no way to handle the the following:
Differentiating between a v1 transaction and a v0 transaction from a txrep. Maybe for practical purposes this isn't a necessity and no one cares as long as the end result of the transaction is the same, but currently there's no way to differentiate between v1 and v0 transactions at the txrep level. There's a
tx.ext.vfield, but currently it is never set in any transactions I've seen (version information is instead at the envelope level, not the transaction itself). So as it stands now, a v1 transaction gets converted to txrep, which when converted back to a transaction becomes a v0 transaction (because there is no way to determine which one to use).There is no way to handle FeeBumpTransactions. txrep specification needs a way to encode information about the outer transaction, otherwise there's just no way to serialize the FeeBumpTransaction to txrep.
Muxed accounts -- I could probably figure this out on my own just by playing with them enough and reading code, but explicitly specifying what changes need to be made to support muxed accounts in txrep would be useful.
The text was updated successfully, but these errors were encountered: