[fastx framework] Maintain object sequence numbers across unwrapping #146
Conversation
sblackshear
requested review from
huitseeker,
lxfind,
patrickkuo,
gdanezis and
oxade
|
This is based on #143, so it will be less noisy to look at only the top commit. |
|
Many thanks for thinking and adding this. It makes things much much simpler of the system side to know that the (objid, seq) will be unique and ever increasing with versions. It allows us, if anything, to maintain efficient causal indexes.
If I understand correctly new objects resulting from a MOVE call start at seq=1. I do not think this is a problem anywhere in our code. I do not believe we assume that the first seq=0, anywhere. (Or at least I cannot think of a fundamental reason we should). |
| @@ -19,7 +28,7 @@ module FastX::ID { | |||
| // TODO (): bring this back once we can support `friend` | |||
| //public(friend) fun new(bytes: vector<u8>): ID { | |||
| public fun new(bytes: address): ID { | |||
| ID { id: IDBytes { bytes } } | |||
| ID { id: IDBytes { bytes }, version: 0 } | |||
| @@ -12,22 +12,90 @@ use crate::{ | |||
| sha3_hash, BcsSignable, FastPayAddress, ObjectDigest, ObjectID, ObjectRef, SequenceNumber, | |||
| TransactionDigest, | |||
| }, | |||
| error::{FastPayError, FastPayResult}, | |||
| gas_coin::GasCoin, | |||
| }; | |||
|
|
|||
| #[derive(Eq, PartialEq, Debug, Clone, Deserialize, Serialize)] | |||
| pub struct MoveObject { | |||
There was a problem hiding this comment.
We use id, version, digest ie object_ref so often it might be beneficial to cache them in a field when we first compute them.
There was a problem hiding this comment.
Yep, I agree (and similar for turning a module object into a CompiledModule)
|
Curious, why do we call it version now instead of just sequence_number? |
Trying approach (1) to addressing MystenLabs/sui#98. In particular, this: - Extends the Move `ID` type to include a `version`. This allows all Move objects to carry their verson, and thus persist it across wrapping and unwrapping. But having it live inside `ID` saves us from having to rewrite the `ID`-related bytecode verifier passes or ask the programmer to write `struct S has key { id: ID, version: Version, ... }` to declare a FastX object. In fact, there are no changes from the programmer's perspective except that they can now read an object's version inside Move if they wish to. - Removed `version` from the Rust object types, since it now lives in the Move-managed `contents` field. Added a variety of helper functions for reading and writing the `version from Rust. - Changed the adapter to understand the new location of `version`. This actually simplifies the adapter code quite a bit. - Added a test that demonstrates that sequence numbers are maintained correctly across wrapping and unwrapping. There is one change introduced here that is important enough to mention separately *object versions now begin at 1*. That is, if a tranaction creates and then transfers an object `X`, the version of `X` in the transaction effects will be 1, not 0. The reasons why this change is needed are somewhat subtle. - This change happens because the adapter increments the sequence number of every transferred object. - You could imagine asking the adapter to instead check whether a transferred object was created by the current transaction, passed as an input, or unwrapped + only incrementing the sequence number in the second two cases. However, if sequence numbers of freshly created objects started at 0, the adapter would actually not be able to tell the difference between a freshly created object `O1` (will have seq 0) and an object `O2` that was created (will have seq 0), then subsquently wrapped (will still have seq 0), and later unwrapped (will still have seq 0!). - Another solution to this problem would be incrementing the sequence number of all objects passed as inputs to the transaction before beginning execution. This would allow freshly created objects to start at seq 0, but it would be slightly odd in that the programmer would pass in an object with seq `S`, but would see `S+1 if they try to read the sequence number from Move during the transaction. It seems most intuitive to maintain the invariant that the object you put into the transaction is exactly what you will read inside the transaction. In addition, that would require us to do the "created or transferred/unwrapped" special-casing described above, which makes the adapter a bit more complicated.
sblackshear
force-pushed
the
preserve_object_version_across_wrapping
branch
from
d5f1f94 to
08589fb
Compare
Yes, good question (and this PR is only going part of the way toward that renaming)... The term "sequence number" has connotations of replay protection, but this is actually not the role of what we call sequence numbers. In FastX, replay protection comes from checking that the inputs And in a related discussion: the keys in this map are currently |
Correct. And because all objects are Move objects, that means every new object starts at seq=1. This is indeed not an assumption that needs to be reflected in our code--everything should work fine if an object has seq 0, it's just that in practice that won't happen. |
* Update README.md Continue edits to Narwhal Benchmarks README * Update README.md * Update README.md Finish text edits to Narwhal Benchmarks README * Update README.md Add PasswordRequiredException warning
* Update README.md Continue edits to Narwhal Benchmarks README * Update README.md * Update README.md Finish text edits to Narwhal Benchmarks README * Update README.md Add PasswordRequiredException warning
Trying approach (1) to addressing #98. In particular, this:
IDtype to include aversion. This allows all Move objects to carry their version, and thus persist it across wrapping and unwrapping. But having it live insideIDsaves us from having to rewrite theID-related bytecode verifier passes or ask the programmer to writestruct S has key { id: ID, version: Version, ... }to declare a FastX object. In fact, there are no changes from the programmer's perspective except that they can now read an object's version inside Move if they wish to.versionfrom the Rust object types, since it now lives in the Move-managedcontentsfield. Added a variety of helper functions for reading and writing the `version from Rust.version. This actually simplifies the adapter code quite a bit.There is one change introduced here that is important enough to mention separately object versions now begin at 1. That is, if a transaction creates and then transfers an object
X, the version ofXin the transaction effects will be 1, not 0. The reasons why this change is needed are somewhat subtle:O1(will have seq 0) and an objectO2that was created (will have seq 0), then subsequently wrapped (will still have seq 0), and later unwrapped (will still have seq 0!).S, but would see `S+1 if they try to read the sequence number from Move during the transaction. It seems most intuitive to maintain the invariant that the object you put into the transaction is exactly what you will read inside the transaction. In addition, that would require us to do the "created or transferred/unwrapped" special-casing described above, which makes the adapter a bit more complicated.