[0/n][transfer-to-object] Transfer to Object Implementation #12611

tzakian · 2023-06-21T19:43:03Z

Description

This PR implements the core transfer-to-object functionality. In particular it implements the ability to "receive" an object that was sent to the address (object ID) of another object using one of the transfer or transfer_public functions in the transfer module.

More detail is given on the programming model in the attached issue so I will not go into that.

SDK support for receiving objects has been added in the two PRs stacked on this one:

[3/n][transfer-to-object] Add SDK support for receiving objects #12987 Adds the Receiving type to the json-rpc types, and adds support receiving objects in the Typescript SDK.
[2/n][transfer-to-object] Add Rust SDK support for receiving arguments #12988 Adds support for receiving objects in the Rust SDK
[2/n][transfer-to-object] Add pruning of object marker table at epoch boundaries #13420 Adds pruning of the per_epoch_object_marker table at epoch boundaries

Test Plan

I've written a number of tests for this that I believe cover things:

Execution-correctness tests for this in the transactional tests
Tests for effect computation in the new sui-core transfer_to_object.rs tests (e.g., receive-then-unwrap, receive-unwrap-wrap, etc).
Tests for lock-freeness of receiving arguments (i.e., that the object identified by the Receiving argument is not locked at signing) in the sui-core transfer_to_object.rs tests
Tests that dependencies are correctly registered, and notified in the transaction manager for Receiving arguments to transactions (see new tests in the transaction_manager_tests.rs file).

A more detailed listing of the tests:

PTBs
- Receive object and return to PTB
  - Do not do anything with the returned (non-drop) value [receive_return_object_dont_touch.move]
  - Call transfer object on it [receive_return_object_then_transfer.move]
- Basic "can receive and then do something in the function" [basic_receive.move]
- Duplicate "Receive" arguments to the PTB [duplicate_receive_argument.move]
- Pass but don't use Receiving argument, then later use it in PTB.
  - By immut ref [pass_receiver_immut_then_reuse.move]
  - By mut ref [pass_receiver_mut_then_reuse.move]
  - By value and returning it [pass_through_then_receive.move]
- Various combinations of receivership being passed [receive_by_ref.move]
  (checking borrow/borrow_mut, and restore rules for PTB execution)
- Receive object of different type [receive_invalid_type.move]
- Receive object with non-address owner ownership [receive_object_owner.move]
- Reuse of input receiving argument [take_receiver_then_try_to_reuse.move]
Type malleability [receive_invalid_param_ty.move]
- Pass receiver into a non-receiver type
  - primitive type
  - struct type with same layout
  - struct type with different layout
- Pass non-receiver into a receiver type
  - primitive type
  - struct type with same layout
  - struct type with different layout
Resource conservation/Effects calculation (both transactional tests and sui-core tests for explicit effects checks)
- Do various things with object after receiving it:
  - Immediately place it as a dynamic field [receive_dof_and_mutate.move]
  - Immediately add a dynamic field to it [receive_add_dof_and_mutate.move]
  - Immediately add a dynamic field to it, add as a dynamic field to parent object, then mutate both [receive_add_dof_and_mutate.move]
  - Immediately transfer it [receive_and_send_back.move]
  - Immediately delete it [receive_and_deleted.move]
  - Immediately wrap it [receive_and_wrap.move]
  - Immediately abort [receive_and_abort.move]
  - Don't use it [receive_by_value_flow_through.move]
  - Receive multiple times in a row making sure effects stay in-sync as expected [receive_multiple_times_in_row.move]
- Shared objects
  - Make sure we can receive if object is transferred to an object which is already shared [shared_parent/basic_receive.move]
  - Make sure we can receive if object is transferred to an object which is then shared [shared_parent/transfer_then_share.move]
  - Non-usage of receiving object argument off a shared parent object [shared_parent/drop_receiving.move]
  - Receive object off of shared parent, add as dynamic field of shared parent and then mutate through the parent [shared_parent/receive_dof_and_mutate.move]
  - Send and receive the same object to the same shared parent multiple times [shared_parent/receive_multiple_times_in_row.move]
- MVCC -- Test that we calculate contained UIDs correctly when we receive an
  object. This is tested in [mvcc/receive_object_dof.move] and
  [mvcc/receive_object_split_changes_dof.move]

Sui core tests checking explicit parts of the calculated effects to make sure they match what we expect:
- Immediately unwrap then transfer inner object [transfer_to_object_tests.rs/test_tto_unwrap_transfer]
- Immediately unwrap then delete inner object as well [transfer_to_object_tests.rs/test_tto_unwrap_delete]
- Immediately unwrap then add inner object as dynamic field [transfer_to_object_tests.rs/test_tto_unwrap_add_as_dynamic_field]
  - Immediately unwrap, then wrap again -- this is part of the above since adding a dynamic field wraps the object
- Basic object receive [transfer_to_object_tests/test_tto_transfer]
- Pass but don't ise Receiving argument [transfer_to_object_tests/test_tto_unused_receiver]
- Pass by different references [transfer_to_object_tests/test_tto_pass_receiving_by_refs]
- Receive and immediately delete [transfer_to_object_tests/test_tto_delete]
- Receive, wrap, and then transfer wrapped object [transfer_to_object_tests/test_tto_wrap]

Sui Core for object locking and transaction dependendency calculation in effects
- Test that receiving object arguments are not locked, and that different
  orders of execution for two certs that want to receive the same argument
  (but only one is valid) can both be run in either order, and both return
  the same execution effects in either order
  [transfer_to_object_tests/test_tt_not_locked]
- Test that transaction dependencies are added correctly:
  - Basic test that we add transaction dependendency if we execute
    successfully and receive the object
    [transfer_to_object_tests/test_tto_valid_dependencies]
  - Similar case for if we delete the object immediately
    [transfer_to_object_tests/test_tto_valid_dependencies_delete_on_receive]
  - That we don't register the transaction dependendency if we don't receive
    the object
    [transfer_to_object_tests/test_tto_dependencies_dont_receive]
  - That we don't register the transaction dependendency if we don't receive
    the object and we abort
    [transfer_to_object_tests/test_tto_dependencies_dont_receive_but_abort]
  - That we register the dependendency if we received the object, even if we
    then went on to abort in the transaction
    [transfer_to_object_tests/test_tto_dependencies_receive_and_abort]
  - Dynamic object field spoofing: make sure we don't accidentally register a
    dynamic object field load of an object that we want to receive at a
    different version as a receivership of that object (i.e., don't register
    the transaction dependendency)
    [transfer_to_object_tests/receive_and_dof_interleave]

Additional tests

PTBs
- MakeMoveVec:
  - create but don't use [receive_many_move_vec.move]
  - pass vec by value but don't receive [receive_many_move_vec.move]
  - pass vec by ref then use value to receive in later command [receive_many_move_vec.move]
  - Pass vec by mut ref and pop/receive some, then receive rest in other call [receive_many_move_vec.move]
  - Pass vec by mut ref, only receive some [receive_many_move_vec.move]
  - Pass vec by value, only receive some [receive_many_move_vec.move]
  - Pass vec by value, receive all [receive_many_move_vec.move]
- Pack receiving tickets into a struct (some/all) then receive transitively [receive_duo_struct.move]
Type mismatches:
- Receiving and phony struct with same struct layout and right type args ([receive_invalid_param_ty.move])
- Receiving with mismatched type args [move_vec_receiving_types.move]
- Receiving with multiple different type args [move_vec_receiving_types.move]
TransferObjects
- Try to transfer receiving ticket [receive_ticket_coin_operations.move]
SplitCoins
- Try to split a receiving ticket [receive_ticket_coin_operations.move]
MergeCoins
- Try to merge a receiving ticket [receive_ticket_coin_operations.move]
MVCC [receive_object_access_through_parent[dof/df].move]
Transaction input checks (in sui-core tests)
- Delete between cert and execution [tests in test_tto_not_lockedin the sui-core tests
- Cert denial if sending a transaction where input_objects \intersect receiving_object != {} [test_tto_intersection_input_and_receiving_objects]
Type-fixing for receiving arguments [pt_receive_type_fixing.move]

If your changes are not user-facing and not a breaking change, you can skip the following section. Otherwise, please indicate what changed, and then add to the Release Notes section as highlighted during the release process.

Type of Change (Check all that apply)

protocol change
user-visible impact
breaking change for a client SDKs
breaking change for FNs (FN binary must upgrade)
breaking change for validators or node operators (must upgrade binaries)
breaking change for on-chain data layout
necessitate either a data wipe or data migration

Release notes

Added the ability to receive objects off of another object. This is currently only turned on in devnet. More information on transfer-to-object, and receiving objects off of other objects can be found in the GitHub issue on it which can be found here: #12658

vercel · 2023-06-21T19:43:12Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
sui-typescript-docs	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Sep 22, 2023 8:17pm

7 Ignored Deployments

Name	Status	Preview	Updated (UTC)
explorer	⬜️ Ignored (Inspect)	Visit Preview	Sep 22, 2023 8:17pm
explorer-storybook	⬜️ Ignored (Inspect)		Sep 22, 2023 8:17pm
multisig-toolkit	⬜️ Ignored (Inspect)	Visit Preview	Sep 22, 2023 8:17pm
mysten-ui	⬜️ Ignored (Inspect)	Visit Preview	Sep 22, 2023 8:17pm
sui-kiosk	⬜️ Ignored (Inspect)	Visit Preview	Sep 22, 2023 8:17pm
sui-wallet-kit	⬜️ Ignored (Inspect)		Sep 22, 2023 8:17pm
wallet-adapter	⬜️ Ignored (Inspect)		Sep 22, 2023 8:17pm

crates/sui-types/src/transaction.rs

crates/sui-core/src/authority/authority_store.rs

crates/sui-json-rpc-types/src/sui_transaction.rs

amnn · 2023-06-23T09:13:53Z

crates/sui-types/src/execution.rs

@@ -205,6 +218,8 @@ impl Value {
                },
                _,
            ) => *used_in_non_entry_move_call,
+            // TODO(tzakian): Need to figure this out.


The only thing you can do with a Receiver<T> is consume it to receive the object, right? So it doesn't seem possible to use the one in a non-entry move call and then have the value live to be queried about it, which implies false is the correct return value here.

sui-execution/latest/sui-adapter/src/programmable_transactions/context.rs

sui-execution/latest/sui-adapter/src/programmable_transactions/execution.rs

lxfind · 2023-06-26T21:50:04Z

crates/sui-types/src/transaction.rs

+        }
+    }
+
+    fn receiving_objects(&self) -> Vec<ObjectRef> {


What's the reason to not include the Receiving in the input objects with a new InputObjectKind, and handle this new kind differently when needed? Asking because I believe there are much more similarities between input objects and receiving objects, than differences.

My first crack at implementing this is exactly as you describe (received objects are returned as input objects with a new InputObjectKind). That direction is definitely workable I believe, however what I encountered when doing that was there were a lot of places where InputObjectKinds flow where Receiving objects make no sense (e.g., in checking or loading object arguments to a transaction). So this led to a bunch of unreachable!/invariant_violation code. @tnowacki had some opinions on this I believe as well (or maybe I'm misremembering).

Moving to separating them out seemed like a cleaner option since it led to significantly less unreachable/invariant_violation code paths and (IMO) seemed to fit more. I actually view Receiving objects as more akin to dynamic fields than input objects other than the addition of dependencies, but I'm more than happy to also prototype treating them as input objects some more and we can take a look at both options and decide if you'd like :)

I think the main thing with not treating Receiving objects as input objects is there are also a number of places where they need to be treated differently from other input objects, especially around the fact that we (1) don't lock them like we do with other objects, and (2) we don't check their existence prior to execution -- in this way they're more akin to dynamic fields with a specified version than an input object. Additionally at the execution layer, Receiving objects and other input objects differ significantly in terms of how they should be treated and checked.

Because of this the worry is that if we treat Receiving objects as input objects, they could accidentally leak to someplace they should and we might, e.g., accidentally grab a lock on them (or try to load them etc.) when we shouldn't. Treating them separately makes this much less likely IMO.

lxfind · 2023-06-26T21:58:01Z

crates/sui-core/src/transaction_input_checker.rs

@@ -68,9 +68,11 @@ pub async fn check_transaction_input(
    transaction.check_version_supported(epoch_store.protocol_config())?;
    transaction.validity_check(epoch_store.protocol_config())?;
    let input_objects = transaction.input_objects()?;
+    let receiving_objects = transaction.receiving_objects()?;


There are several similar functions in this file that checks input for different kinds of transactions. It's an unfortunate duplication of code but it does mean we have to handle receiving objects in all those places. This further backs my question regarding whether we should include receiving objects in input objects, otherwise you would have to chase down all calls to input_objects().

My understanding is that receiving objects can only be used for PTBs (outside of those they don't make any sense I believe). So I don't believe we need to handle these separate apart from possibly checking that the set of receiving_objects is empty in that case.

Not sure if I understand, but check_transaction_input_with_given_gas and check_dev_inspect_input could be called on PTBs as well?

ahh correct, these will need to be added there as well.

@laura-makdah

…n and transfer-to-object (#12852) ## Description This adds a new table to the ``PerpetualTables` that holds the set of received and shared deleted objects during the epoch. This table is not currently used anywhere and is unused. However, both shared-object deletion #12623 and transfer-to-object #12611 PRs rely on this table. This table can be pruned at epoch boundaries so the table key contains the epoch ID. @laura-makdah and I have discussed and we're happy to either land this soon, or to wait until one or the other above PRs is ready to land and land this change at the same time. Either way this change on its own should be fine to land any time as the new table is not used anywhere yet. ## Test Plan Not tested yet -- functionality will be tested in the two PRs being built on top of this. --- If your changes are not user-facing and not a breaking change, you can skip the following section. Otherwise, please indicate what changed, and then add to the Release Notes section as highlighted during the release process. ### Type of Change (Check all that apply) - [ ] protocol change - [ ] user-visible impact - [ ] breaking change for a client SDKs - [ ] breaking change for FNs (FN binary must upgrade) - [ ] breaking change for validators or node operators (must upgrade binaries) - [ ] breaking change for on-chain data layout - [ ] necessitate either a data wipe or data migration ### Release notes

@laura-makdah

…n and transfer-to-object (#12852) ## Description This adds a new table to the ``PerpetualTables` that holds the set of received and shared deleted objects during the epoch. This table is not currently used anywhere and is unused. However, both shared-object deletion #12623 and transfer-to-object #12611 PRs rely on this table. This table can be pruned at epoch boundaries so the table key contains the epoch ID. @laura-makdah and I have discussed and we're happy to either land this soon, or to wait until one or the other above PRs is ready to land and land this change at the same time. Either way this change on its own should be fine to land any time as the new table is not used anywhere yet. ## Test Plan Not tested yet -- functionality will be tested in the two PRs being built on top of this. --- If your changes are not user-facing and not a breaking change, you can skip the following section. Otherwise, please indicate what changed, and then add to the Release Notes section as highlighted during the release process. ### Type of Change (Check all that apply) - [ ] protocol change - [ ] user-visible impact - [ ] breaking change for a client SDKs - [ ] breaking change for FNs (FN binary must upgrade) - [ ] breaking change for validators or node operators (must upgrade binaries) - [ ] breaking change for on-chain data layout - [ ] necessitate either a data wipe or data migration ### Release notes

tnowacki

Continuing to review, but giving feedback as immediately as I can. So expect some more comments over the afternoon here. But the receiving value should not need to use global value, because we are protecting the loading by the uniqueness of the Receiving value

tnowacki · 2023-07-13T22:17:56Z

sui-execution/v0/sui-adapter/src/programmable_transactions/context.rs

@@ -1216,6 +1222,7 @@ fn load_object_arg<'vm, 'state>(
            /* imm override */ !mutable,
            id,
        ),
+        ObjectArg::Receiving(_) => invariant_violation!("Impossible to hit Receiving in v0"),


How do these things work? Should we just panic as we will otherwise fork? Not sure I have personally made such a decision yet, so mostly wondering what is standard for myself :)

Same! I think panicking might be the best call here since that would be better. @amnn any thoughts on this?

sui-execution/latest/sui-verifier/src/private_generics.rs

tnowacki · 2023-07-13T22:21:36Z

sui-execution/latest/sui-move-natives/src/transfer.rs

+    let Some((tag, layout, annotated_layout)) = get_tag_and_layouts(context, &child_ty)?  else {
+        return Ok(NativeResult::err(
+                context.gas_used(),
+                E_BCS_SERIALIZATION_FAILURE,


Is this not an invariant violation?

I was going off of and replicating similar error handling as in the get_or_fetch_object macro. But happy to make it an invariant violation (storage error).

sui-execution/latest/sui-move-natives/src/object_runtime/object_store.rs

sui-execution/latest/sui-move-natives/src/dynamic_field.rs

sui-execution/latest/sui-adapter/src/programmable_transactions/execution.rs

tnowacki · 2023-07-13T23:20:41Z

sui-execution/latest/sui-adapter/src/programmable_transactions/context.rs

+            // We don't count receiving object arguments as something that needs to be written
+            // unless it has actually been received -- this is tracked in the `received` field of
+            // the object store.
+            if let InputObjectMetadata::Receiving { .. } = &object_metadata { return Ok(()) }


Shouldn't we early return above if we are in this case? Then we shouldn't need to clone.

(you might need to copy out the owner/id fields again but should be easy enough

tnowacki · 2023-07-13T23:22:22Z

sui-execution/latest/sui-adapter/src/programmable_transactions/context.rs

+                let old_value = value_opt.replace(Value::Receiving(*id, *version));
+                // If the value is receiving and it's being restored it must have been taken since
+                // the `Receiving` struct is not copy.
+                assert_invariant!(


restore is used to implement borrow_mut, so I think we will be able to hit this case

I don't think we're able to hit this case since it doesn't have copy -- if we mutably borrow the value will be taken and then restored so old_value should always be none

Ah, I see. Makes sense (just be sure to test it ;))

.../sui-verifier-transactional-tests/tests/private_generics/receive_with_and_without_store.mvir

crates/sui-types/src/transfer.rs

tnowacki · 2023-07-13T23:34:45Z

crates/sui-types/src/transaction.rs

+            CallArg::Object(o) => match o {
+                ObjectArg::ImmOrOwnedObject(_) => vec![],
+                ObjectArg::SharedObject { .. } => vec![],
+                ObjectArg::Receiving(obj_ref) => vec![*obj_ref],


Why is this a vec instead of just an option?

Don't have a super string opinion here, but I used a vec instead of an option here to keep consistency with the other interfaces around receiving objects, and input objects on PTB/PTB components. In particular both input_objects and with other implementations of receiving_objects on other parts of the PTB.

crates/sui-types/src/temporary_store.rs

crates/sui-core/src/authority/authority_store.rs

tnowacki · 2023-09-22T17:15:52Z

crates/sui-adapter-transactional-tests/tests/receive_object/pt_receive_type_fixing.move

+    public entry fun pass_through(x: Receiving<B>): Receiving<B> { x }
+
+    public entry fun pass_through_a(x: Receiving<A>): Receiving<A> { x }
+
+    public entry fun pass_through_ref_a(_x: &Receiving<A>) { }
+
+    public entry fun pass_through_mut_ref_a(_x: &mut Receiving<A>) { }


Did we end up fixing the type on first usage? Or first mutable usage?
If the latter, I have some more tests for you :)

We did windup fixing the type on first usage!

tnowacki · 2023-09-22T17:16:37Z

crates/sui-adapter-transactional-tests/tests/receive_object/receive_invalid_param_ty.exp

+task 10 'run'. lines 61-61:
+Error: Error checking transaction input objects: IncorrectUserSignature { error: "Object 0xd38c9db0c1935b185ec59bacb9e6ce2e736579cedabb791fce9a8165cef6c319 is owned by account address 0xa0027b0c13d66e2958feb9c56af104ea20ddfcb46a3ea93b9ba5d3c2d9b94d42, but given owner/signer address is 0xfccc9a421bbb13c1a66a1aa98f0ad75029ede94857779c6915b44f94068b921e" }


Why do we get this error for this one?

This is an object-owned object that I am trying to pass-in as a normal object to the transaction, so the transaction input checks are going to complain about this (with this error).

lxfind

Question: when a transaction receives an object, and deletes (or wraps) it in the same transaction, which code ensures that it's marked as modified?

lxfind · 2023-09-22T17:19:24Z

sui-execution/vm-rework/sui-adapter/src/gas_charger.rs

@@ -207,6 +209,14 @@ pub mod checked {
            self.gas_status.charge_publish_package(size)
        }

+        pub fn charge_upgrade_package(&mut self, size: usize) -> Result<(), ExecutionError> {


Seems unrelated code?

Bad rebase from @dariorussi's fix maybe?

The vm_rework branch was cut before Dario's fix, so when I rebased the changes from latest into vm_rework it pulled in the changes to latest with it. So I think this is fine/to be expected?

Happy to not replicate it over, but it feels kinda weird to not just do a full execution_layer.py rebase to replicate the changes over.

tzakian · 2023-09-22T17:42:48Z

@lxfind

Question: when a transaction receives an object, and deletes (or wraps) it in the same transaction, which code ensures that it's marked as modified?

Good question, this is done by marking every received object as mutated when we finish off the ObjectRuntimeState:

~~https://github.com/MystenLabs/sui/pull/12611/files#diff-35345cac32d6832985925d47d02cf4d4ef34ed75330e1a94eb5cb50a03952beaR578~~

Edit: apparently github doesn't support sending lines to a PR, even though it seems like it should... here's a link to the line in question that should work:

sui/sui-execution/latest/sui-move-natives/src/object_runtime/mod.rs

Line 580 in 7128bab

for (received_object, _) in received.into_iter() {

lxfind · 2023-09-22T17:45:19Z

sui-execution/latest/sui-move-natives/src/object_runtime/mod.rs

@@ -531,8 +575,10 @@ impl ObjectRuntimeState {
                (id, (owner, type_, value))
            })
            .collect();
-        for deleted_id in deleted_ids.keys() {


Woooo I don't think we can delete this yet. This is required at least prior to @tnowacki's fix on the recent child object issue.

Good catch! Must have happened in a bad rebase...I'll add this back in!

There were no tests that caught this? would it be possible to add one?

lxfind

Overall LGTM on the core changes

lxfind · 2023-09-22T18:19:44Z

sui-execution/latest/sui-move-natives/src/object_runtime/mod.rs

@@ -537,6 +587,13 @@ impl ObjectRuntimeState {
            }
        }

+        // Any received objects are viewed as modified.
+        for (received_object, _) in received.into_iter() {
+            if let Some(loaded_child) = loaded_child_objects.get_mut(&received_object) {


Should it be an invariant violation if it's not loaded?

mystenmark · 2023-09-22T17:54:05Z

crates/sui-core/src/authority/authority_store.rs

+            if has_key {
+                versioned_results.push((*idx, true))
+            } else if receiving_objects.contains(input_key) {
+                // If there's a more recent version of this object, then it was mutated by someone


I don't understand this - the comment here implies that this branch only enters if there was a more recent version of the object. But I don't see why has_key can't also be true when a more recent version exists.

Good call out -- I'll update the comments to clarify.

You're correct that has_key could also be true if a more recent version exists (and that's fine).

What I really should say here (and I'll correct to this) is that has_key can also return false if a more recent version exists in the case where the object at the specified version has been pruned. In that case we need to "look up" to see if a more recent version of that object has been exists or has been deleted.

mystenmark · 2023-09-22T18:11:15Z

crates/sui-core/src/authority/authority_store.rs

@@ -1884,6 +2046,35 @@ impl ChildObjectResolver for AuthorityStore {
        }
        Ok(Some(child_object))
    }
+
+    fn get_object_received_at_version(


where is this actually called from?

All of the calls from the object runtime to receive an object call this method. E.g.,

sui/sui-execution/latest/sui-move-natives/src/object_runtime/object_store.rs

Line 90 in f225f58

.get_object_received_at_version(&owner, &child, version, self.current_epoch_id)

uses this to do the actual loading of the object while executing the transaction.

omg I was grepping in /crates so I didn't see those.

I've been there...more times than I'd care to admit

mystenmark · 2023-09-22T18:23:56Z

sui-execution/latest/sui-adapter/src/temporary_store.rs

+            // If the receiving object is in the loaded runtime objects, then that means that it
+            // was actually successfully loaded (so existed, and there was authenticated mutable
+            // access to it). So we insert the previous transaction as a dependency.


Why? This just seems correct?

mystenmark · 2023-09-22T18:26:41Z

sui-execution/latest/sui-adapter/src/temporary_store.rs

+            if let Some(obj_meta) = self.loaded_runtime_objects.get(id) {
+                // Check that the expected version, digest, and owner match the loaded version,
+                // digest, and owner. If they don't then don't register a dependency.
+                // This is because this could be "spoofed" by loading a dynamic object field.


can you explain this spoofing? shouldn't an attempt to receive a child object fail because the object will be object owned?

Basically, due to the way dynamic object fields work, you can have the following sequence of certificates and executions:

Object O at version n with object id oid is sent to a shared object S

Tx1 creates a cert to receive O at version n but never actually tries to receive O. BUT Tx1 loads a dynamic object field F that will reference this object in the future. (Tx1 could not be executed successfully right now).

Tx2 creates a cert, and executes a transaction moving O to the dynamic object field F. O now has an ObjectOwner() ownership, but importantly the same object id oid.

Tx1 executes. This will load O via the dynamic object field at a different version n + k (k > 0). Because of this oid will be in the loaded_runtime_objects but at a different version.

If we don't have this check that the object in the loaded runtime object metadata was actually received at the specified version (and I added the other checks just to be doubly sure) then we could register a transaction dependency that actually didn't exist. Not a massive deal, but also not quite correct either.

A test that demonstrates this is here:

sui/crates/sui-core/src/unit_tests/transfer_to_object_tests.rs

Line 1531 in f225f58

async fn receive_and_dof_interleave() {

…13896) ## Description This adds support for receiving transaction argument types to json-rpc ## Test Plan Tested by PRs above this. --- If your changes are not user-facing and not a breaking change, you can skip the following section. Otherwise, please indicate what changed, and then add to the Release Notes section as highlighted during the release process. ### Type of Change (Check all that apply) - [X] protocol change - [X] user-visible impact - [ ] breaking change for a client SDKs - [ ] breaking change for FNs (FN binary must upgrade) - [ ] breaking change for validators or node operators (must upgrade binaries) - [ ] breaking change for on-chain data layout - [ ] necessitate either a data wipe or data migration ### Release notes Release notes included in base PR.

Synched out-of-band

tzakian requested review from amnn, lxfind, tnowacki and mystenmark June 21, 2023 19:43

vercel bot had a problem deploying to Preview – multisig-toolkit June 21, 2023 19:43 Failure

tzakian force-pushed the tzakian/tto-1 branch from 08c42d5 to 6edd778 Compare June 21, 2023 19:45

vercel bot had a problem deploying to Preview – multisig-toolkit June 21, 2023 19:46 Failure

tzakian requested a review from lanvidr June 21, 2023 19:48

tzakian force-pushed the tzakian/tto-1 branch from 6edd778 to 28c9b5f Compare June 22, 2023 22:52

vercel bot had a problem deploying to Preview – multisig-toolkit June 22, 2023 22:53 Failure

mystenmark reviewed Jun 22, 2023

View reviewed changes

crates/sui-types/src/transaction.rs Show resolved Hide resolved

crates/sui-core/src/authority/authority_store.rs Outdated Show resolved Hide resolved

crates/sui-core/src/authority/authority_store.rs Outdated Show resolved Hide resolved

amnn reviewed Jun 23, 2023

View reviewed changes

tzakian linked an issue Jun 23, 2023 that may be closed by this pull request

Transfer to Object #12658

Closed

tzakian force-pushed the tzakian/tto-1 branch from 28c9b5f to 9e90813 Compare June 24, 2023 01:00

lxfind reviewed Jun 26, 2023

View reviewed changes

tzakian force-pushed the tzakian/tto-1 branch 2 times, most recently from c9eb4ee to bbb4c95 Compare June 28, 2023 19:10

tzakian changed the title ~~[RFC][transfer-to-object] Initial plumbing of InboxRef transaction argument type~~ [RFC][transfer-to-object] Transfer to Object Implementation Jul 5, 2023

tzakian mentioned this pull request Jul 5, 2023

[storage] Add per-epoch object marker table for shared object deletion and transfer-to-object #12852

Merged

7 tasks

tzakian force-pushed the tzakian/tto-1 branch 2 times, most recently from 84c9e1f to b889c94 Compare July 11, 2023 21:10

tzakian changed the title ~~[RFC][transfer-to-object] Transfer to Object Implementation~~ [transfer-to-object] Transfer to Object Implementation Jul 11, 2023

tzakian force-pushed the tzakian/tto-1 branch from b889c94 to 0085e91 Compare July 13, 2023 20:37

tzakian mentioned this pull request Jul 13, 2023

[2/n][transfer-to-object] Add Rust SDK support for receiving arguments #12988

Merged

7 tasks

tnowacki previously requested changes Jul 13, 2023

View reviewed changes

tnowacki reviewed Jul 13, 2023

View reviewed changes

tzakian force-pushed the tzakian/tto-1 branch from 0085e91 to 7e5c518 Compare July 15, 2023 00:24

vercel bot temporarily deployed to Preview – mysten-ui September 21, 2023 02:58 Inactive

tzakian requested a review from tnowacki September 21, 2023 03:10

tzakian force-pushed the tzakian/tto-1 branch from 860849c to ae242b4 Compare September 21, 2023 03:15

vercel bot deployed to Preview – sui-typescript-docs September 21, 2023 03:15 View deployment

vercel bot temporarily deployed to Preview – mysten-ui September 21, 2023 03:16 Inactive

tzakian force-pushed the tzakian/tto-1 branch from ae242b4 to a7c90d9 Compare September 22, 2023 01:22

vercel bot deployed to Preview – sui-typescript-docs September 22, 2023 01:22 View deployment

vercel bot temporarily deployed to Preview – mysten-ui September 22, 2023 01:23 Inactive

tzakian force-pushed the tzakian/tto-1 branch from a7c90d9 to 7128bab Compare September 22, 2023 17:08

vercel bot deployed to Preview – sui-typescript-docs September 22, 2023 17:08 View deployment

vercel bot temporarily deployed to Preview – mysten-ui September 22, 2023 17:08 Inactive

tnowacki reviewed Sep 22, 2023

View reviewed changes

lxfind reviewed Sep 22, 2023

View reviewed changes

tzakian added 3 commits September 22, 2023 11:14

Core transfer-to-object-changes

0109891

[0/n][transfer-to-object] Transfer to object tests

1996599

Additional Tests

f225f58

tzakian force-pushed the tzakian/tto-1 branch from 7128bab to f225f58 Compare September 22, 2023 18:17

vercel bot deployed to Preview – sui-typescript-docs September 22, 2023 18:17 View deployment

vercel bot temporarily deployed to Preview – mysten-ui September 22, 2023 18:17 Inactive

lxfind approved these changes Sep 22, 2023

View reviewed changes

mystenmark reviewed Sep 22, 2023

View reviewed changes

Address comments

e81934c

vercel bot deployed to Preview – sui-typescript-docs September 22, 2023 19:04 View deployment

vercel bot deployed to Preview – sui-typescript-docs September 22, 2023 20:17 View deployment

tzakian merged commit e70909d into main Sep 22, 2023

tzakian deleted the tzakian/tto-1 branch September 22, 2023 21:56

		task 10 'run'. lines 61-61:
		Error: Error checking transaction input objects: IncorrectUserSignature { error: "Object 0xd38c9db0c1935b185ec59bacb9e6ce2e736579cedabb791fce9a8165cef6c319 is owned by account address 0xa0027b0c13d66e2958feb9c56af104ea20ddfcb46a3ea93b9ba5d3c2d9b94d42, but given owner/signer address is 0xfccc9a421bbb13c1a66a1aa98f0ad75029ede94857779c6915b44f94068b921e" }

[0/n][transfer-to-object] Transfer to Object Implementation #12611

[0/n][transfer-to-object] Transfer to Object Implementation #12611

Conversation

tzakian commented Jun 21, 2023 • edited Loading

Description

Test Plan

Additional tests

Type of Change (Check all that apply)

Release notes

vercel bot commented Jun 21, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tnowacki left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lxfind left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tzakian commented Sep 22, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lxfind left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tzakian commented Jun 21, 2023 •

edited

Loading

vercel bot commented Jun 21, 2023 •

edited

Loading

lxfind left a comment •

edited

Loading

tzakian commented Sep 22, 2023 •

edited

Loading