[json] Add modified_at_versions to SuiTransactionEffects #7237
Conversation
tnowacki
commented
Jan 9, 2023
tnowacki
commented
- Added missing field present in TransactionEffects but not for the JSON copy of it
- Added missing field present in TransactionEffects but not for the JSON copy of it
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ |
| // The version that every modified (mutated or deleted) object had before it was modified by | ||
| // this transaction. | ||
| #[serde(default, skip_serializing_if = "Vec::is_empty")] | ||
| pub modified_at_versions: Vec<(ObjectID, SequenceNumber)>, |
There was a problem hiding this comment.
This style, the pair (ObjectID, SequenceNumber), does not seem in line with the rest of this struct, but I was unsure if I should make a type just for this case.
Any advice @patrickkuo?
There was a problem hiding this comment.
We use custom type (e.g. SuiObjectRef) to give the values names before serialisation to make it more readable.
IIRC this is preferred in typescript? CC @666lcz for comments
| @@ -1916,6 +1920,7 @@ impl SuiTransactionEffects { | |||
| Ok(Self { | |||
| status: effect.status.into(), | |||
| gas_used: effect.gas_used.into(), | |||
| modified_at_versions: effect.modified_at_versions, | |||
There was a problem hiding this comment.
Separate issue, but wondering why the existing modified_at_versions doesn't include the input object digest. This is important info if you want to replay the tx by reading the input values from an untrusted store.
There was a problem hiding this comment.
Originally modified_at_versions was included to support reverting state updates, which assume a trusted store (the validator's own), so that's the reason for the omission. We chatted about this question in a recent meeting, and while this isn't necessary to support querying object history, it will be for replaying a transaction against an untrusted store -- if that's something that we support, we should add the digest.
|
Broader fixes here will be a part of #6529 |
