Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Transaction meta is missing one time signer changes in older versions of the protocol #2217

Closed
MonsieurNicolas opened this issue Aug 7, 2019 · 3 comments · Fixed by #2315

Comments

@MonsieurNicolas
Copy link
Contributor

When removeUsedOneTimeSignerKeys was added in
840787d#diff-94d5ac6726dabe26918718d13fd44b9cR540

the meta format was not updated to reflect those changes (this would have required a "post transaction" meta of sorts).

This was later fixed in protocol 10 (presigned transactions changes are tracked in txChanges as they are done before applying operations):
#1464 (comment)

Proposed fix:
in order to generate meta that contains all changes, a simple update to pre-protocol 10 ledgers would be to modify the meta of the last operation to include the one time signer change (NB: this is a merge of 2 meta).
It's a bit ugly as technically speaking the change doesn't belong to the last operation, but it seems to be the simplest we can do without adding "post tx" meta.

@MonsieurNicolas
Copy link
Contributor Author

Tagging as "discussion" for now. @jonjove FYI

@jonjove
Copy link
Contributor

jonjove commented Oct 15, 2019

I think the best approach is to update the XDR with

struct TransactionMetaV2
{
    LedgerEntryChanges txChangesBefore;
    OperationMeta operations<>;
    LedgerEntryChanges txChangesAfter;
};

union TransactionMeta switch (int v)
{
case 0:
    OperationMeta operations<>;
case 1:
    TransactionMetaV1 v1;
case 2:
    TransactionMetaV2 v2;
};

The contents of TransactionMetaV2.txChangesBefore and TransactionMetaV2.operations would be identical to the contents of TransactionMetaV1.txChanges and TransactionMetaV1.operations. TransactionMetaV2.txChangesAfter would contain the changes for the one-time signer changes up to version 9, and would be empty in versions 10 and beyond.

In order to avoid breaking downstream systems, we would also add a configuration flag SUPPORTED_META_VERSION with default value 1 and the only other acceptable value 2. If SUPPORTED_META_VERSION = 1 then stellar-core emits TransactionMetaV1 and analogously if SUPPORTED_META_VERSION = 2 then stellar-core emits TransactionMetaV2. The intention would be to deprecate generation of TransactionMetaV1 in a few (perhaps 3?) months, so we should also emit a deprecation warning on start-up if SUPPORTED_META_VERSION = 1.

@MonsieurNicolas
Copy link
Contributor Author

@ire-and-curses FYI

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants