[dry run][dev inspect] Fix dynamic field edge case (MystenLabs#10116)

## Description 

Set gas objects in dry run and dev-inspect to the max version to avoid
dynamic field issues we have seen with faucet (and faucet tests).
In dry run and dev inspect, you might load a dynamic field that is
actually too new for the transaction. Ideally, we would want to load the
"correct" dynamic fields, but as that is not easily determined, we
instead can set the sequence number of the gas objects to MAX - 1, to
ensure that the lamport version for the transaction is MAX, and as such,
a valid lamport version for any object used in the transaction
(preventing internal assertions or invariant violations from being
triggered)

## Test Plan 

- Added new tests

---
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)

- [ ] 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

.github/workflows/e2e.yml

@@ -52,14 +52,7 @@ jobs:
 
       - name: Run TS SDK e2e tests
         if: ${{ needs.diff.outputs.isTypescriptSDK == 'true' || needs.diff.outputs.isRust == 'true'}}
-        # usage https://github.com/marketplace/actions/retry-step
-        # TODO: remove this after https://mysten-labs.slack.com/archives/C04FG4Q7YJ3/p1679676070896019 is resolved
-        # retry is added because of flakiness in sui-test-validator dry-run
-        uses: nick-fields/retry@v2
-        with:
-          max_attempts: 5
-          timeout_minutes: 30
-          command: pnpm dlx concurrently --kill-others --success command-1 "$E2E_RUN_LOCAL_NET_CMD" 'pnpm sdk test:e2e'
+        run: pnpm dlx concurrently --kill-others --success command-1 "$E2E_RUN_LOCAL_NET_CMD" 'pnpm sdk test:e2e'
 
       - name: Run Explorer e2e tests
         # need to run Explorer e2e when its upstream(TS SDK and Rust) or itself is changed

crates/sui-core/src/authority.rs

@@ -1054,7 +1054,7 @@ impl AuthorityState {
         let shared_object_refs = input_objects.filter_shared_objects();
 
         let transaction_dependencies = input_objects.transaction_dependencies();
-        let temporary_store = TemporaryStore::new(
+        let temporary_store = TemporaryStore::new_for_mock_transaction(
             self.database.clone(),
             input_objects,
             transaction_digest,
@@ -1166,7 +1166,7 @@ impl AuthorityState {
         let transaction_digest = TransactionDigest::new(default_hash(&data));
         let transaction_kind = data.into_kind();
         let transaction_dependencies = input_objects.transaction_dependencies();
-        let temporary_store = TemporaryStore::new(
+        let temporary_store = TemporaryStore::new_for_mock_transaction(
             self.database.clone(),
             input_objects,
             transaction_digest,

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

@@ -925,6 +925,179 @@ async fn test_dry_run_on_validator() {
     assert!(response.is_err());
 }
 
+// Tests using a dynamic field that a is newer than the parent in dev inspect/dry run
+#[tokio::test]
+async fn test_dry_run_dev_inspect_dynamic_field_too_new() {
+    let (sender, sender_key): (_, AccountKeyPair) = get_key_pair();
+    let gas_object_id = ObjectID::random();
+    let (validator, fullnode) = init_state_validator_with_fullnode().await;
+    let (validator, object_basics) = publish_object_basics(validator).await;
+    let (fullnode, _object_basics) = publish_object_basics(fullnode).await;
+    let gas_object = Object::with_id_owner_for_testing(gas_object_id, sender);
+    let gas_object_ref = gas_object.compute_object_reference();
+    validator.insert_genesis_object(gas_object.clone()).await;
+    fullnode.insert_genesis_object(gas_object).await;
+    // create the parent
+    let effects = call_move_(
+        &validator,
+        Some(&fullnode),
+        &gas_object_id,
+        &sender,
+        &sender_key,
+        &object_basics.0,
+        "object_basics",
+        "create",
+        vec![],
+        vec![
+            TestCallArg::Pure(bcs::to_bytes(&(16_u64)).unwrap()),
+            TestCallArg::Pure(bcs::to_bytes(&sender).unwrap()),
+        ],
+        false,
+    )
+    .await
+    .unwrap();
+    assert_eq!(effects.status(), &ExecutionStatus::Success);
+    assert_eq!(effects.created().len(), 1);
+    let parent = effects.created()[0].0;
+
+    // create the child
+    let effects = call_move_(
+        &validator,
+        Some(&fullnode),
+        &gas_object_id,
+        &sender,
+        &sender_key,
+        &object_basics.0,
+        "object_basics",
+        "create",
+        vec![],
+        vec![
+            TestCallArg::Pure(bcs::to_bytes(&(32_u64)).unwrap()),
+            TestCallArg::Pure(bcs::to_bytes(&sender).unwrap()),
+        ],
+        false,
+    )
+    .await
+    .unwrap();
+    assert_eq!(effects.status(), &ExecutionStatus::Success);
+    assert_eq!(effects.created().len(), 1);
+    let child = effects.created()[0].0;
+
+    // add/wrap the child
+    let effects = call_move_(
+        &validator,
+        Some(&fullnode),
+        &gas_object_id,
+        &sender,
+        &sender_key,
+        &object_basics.0,
+        "object_basics",
+        "add_field",
+        vec![],
+        vec![TestCallArg::Object(parent.0), TestCallArg::Object(child.0)],
+        false,
+    )
+    .await
+    .unwrap();
+    assert_eq!(effects.status(), &ExecutionStatus::Success);
+    assert_eq!(effects.created().len(), 1);
+    let field = effects.created()[0].0;
+
+    // make sure the parent was updated
+    let new_parent = fullnode.get_object(&parent.0).await.unwrap().unwrap();
+    assert!(parent.1 < new_parent.version());
+
+    // delete the child, but using the old version of the parent
+    let pt = ProgrammableTransaction {
+        inputs: vec![CallArg::Object(ObjectArg::ImmOrOwnedObject(parent))],
+        commands: vec![Command::MoveCall(Box::new(ProgrammableMoveCall {
+            package: object_basics.0,
+            module: Identifier::new("object_basics").unwrap(),
+            function: Identifier::new("remove_field").unwrap(),
+            type_arguments: vec![],
+            arguments: vec![Argument::Input(0)],
+        }))],
+    };
+    let kind = TransactionKind::programmable(pt.clone());
+    // dev inspect
+    let DevInspectResults { effects, .. } = fullnode
+        .dev_inspect_transaction_block(sender, kind, Some(1))
+        .await
+        .unwrap();
+    assert_eq!(effects.deleted().len(), 1);
+    let deleted = &effects.deleted()[0];
+    assert_eq!(field.0, deleted.object_id);
+    assert_eq!(deleted.version, SequenceNumber::MAX);
+
+    // dry run
+    let data = TransactionData::new_programmable_with_dummy_gas_price(
+        sender,
+        vec![gas_object_ref],
+        pt,
+        MAX_GAS,
+    );
+    let transaction = to_sender_signed_transaction(data.clone(), &sender_key);
+    let digest = *transaction.digest();
+    let DryRunTransactionBlockResponse { effects, .. } =
+        fullnode.dry_exec_transaction(data, digest).await.unwrap().0;
+    assert_eq!(effects.deleted().len(), 1);
+    let deleted = &effects.deleted()[0];
+    assert_eq!(field.0, deleted.object_id);
+    assert_eq!(deleted.version, SequenceNumber::MAX);
+}
+
+// tests using a gas coin with version MAX - 1
+#[tokio::test]
+async fn test_dry_run_dev_inspect_max_gas_version() {
+    let (sender, sender_key): (_, AccountKeyPair) = get_key_pair();
+    let gas_object_id = ObjectID::random();
+    let (validator, fullnode) = init_state_validator_with_fullnode().await;
+    let (validator, object_basics) = publish_object_basics(validator).await;
+    let (fullnode, _object_basics) = publish_object_basics(fullnode).await;
+    let gas_object = Object::with_id_owner_version_for_testing(
+        gas_object_id,
+        SequenceNumber::from_u64(SequenceNumber::MAX.value() - 1),
+        sender,
+    );
+    let gas_object_ref = gas_object.compute_object_reference();
+    validator.insert_genesis_object(gas_object.clone()).await;
+    fullnode.insert_genesis_object(gas_object).await;
+
+    let pt = ProgrammableTransaction {
+        inputs: vec![
+            CallArg::Pure(bcs::to_bytes(&(32_u64)).unwrap()),
+            CallArg::Pure(bcs::to_bytes(&sender).unwrap()),
+        ],
+        commands: vec![Command::MoveCall(Box::new(ProgrammableMoveCall {
+            package: object_basics.0,
+            module: Identifier::new("object_basics").unwrap(),
+            function: Identifier::new("create").unwrap(),
+            type_arguments: vec![],
+            arguments: vec![Argument::Input(0), Argument::Input(1)],
+        }))],
+    };
+    let kind = TransactionKind::programmable(pt.clone());
+    // dev inspect
+    let DevInspectResults { effects, .. } = fullnode
+        .dev_inspect_transaction_block(sender, kind, Some(1))
+        .await
+        .unwrap();
+    assert_eq!(effects.status(), &SuiExecutionStatus::Success);
+
+    // dry run
+    let data = TransactionData::new_programmable_with_dummy_gas_price(
+        sender,
+        vec![gas_object_ref],
+        pt,
+        MAX_GAS,
+    );
+    let transaction = to_sender_signed_transaction(data.clone(), &sender_key);
+    let digest = *transaction.digest();
+    let DryRunTransactionBlockResponse { effects, .. } =
+        fullnode.dry_exec_transaction(data, digest).await.unwrap().0;
+    assert_eq!(effects.status(), &SuiExecutionStatus::Success);
+}
+
 #[tokio::test]
 async fn test_handle_transfer_transaction_bad_signature() {
     let (sender, sender_key): (_, AccountKeyPair) = get_key_pair();

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

@@ -80,3 +80,39 @@ pub enum ObjectChange {
         digest: ObjectDigest,
     },
 }
+
+impl ObjectChange {
+    pub fn object_id(&self) -> ObjectID {
+        match self {
+            ObjectChange::Published { package_id, .. } => *package_id,
+            ObjectChange::Transferred { object_id, .. }
+            | ObjectChange::Mutated { object_id, .. }
+            | ObjectChange::Deleted { object_id, .. }
+            | ObjectChange::Wrapped { object_id, .. }
+            | ObjectChange::Created { object_id, .. } => *object_id,
+        }
+    }
+
+    pub fn mask_for_test(&mut self, new_version: SequenceNumber, new_digest: ObjectDigest) {
+        match self {
+            ObjectChange::Published {
+                version, digest, ..
+            }
+            | ObjectChange::Transferred {
+                version, digest, ..
+            }
+            | ObjectChange::Mutated {
+                version, digest, ..
+            }
+            | ObjectChange::Created {
+                version, digest, ..
+            } => {
+                *version = new_version;
+                *digest = new_digest
+            }
+            ObjectChange::Deleted { version, .. } | ObjectChange::Wrapped { version, .. } => {
+                *version = new_version
+            }
+        }
+    }
+}

crates/sui-json-rpc/src/unit_tests/rpc_server_tests.rs

@@ -5,6 +5,7 @@ use crate::api::{
     CoinReadApiClient, GovernanceReadApiClient, IndexerApiClient, ReadApiClient,
     TransactionBuilderClient, WriteApiClient,
 };
+use std::collections::BTreeMap;
 use std::path::Path;
 #[cfg(not(msim))]
 use std::str::FromStr;
@@ -25,7 +26,9 @@ use sui_keys::keystore::{AccountKeystore, FileBasedKeystore, Keystore};
 use sui_macros::sim_test;
 use sui_types::balance::Supply;
 use sui_types::base_types::ObjectID;
+use sui_types::base_types::SequenceNumber;
 use sui_types::coin::{TreasuryCap, COIN_MODULE_NAME};
+use sui_types::digests::ObjectDigest;
 use sui_types::gas_coin::GAS;
 use sui_types::messages::ExecuteTransactionRequestType;
 use sui_types::utils::to_sender_signed_transaction;
@@ -128,19 +131,40 @@ async fn test_public_transfer_object() -> Result<(), anyhow::Error> {
         )
         .await?;
 
-    let SuiTransactionBlockResponse {
-        effects,
-        object_changes,
-        ..
-    } = tx_response;
-    assert_eq!(
-        dryrun_response.effects.transaction_digest(),
-        effects.unwrap().transaction_digest()
+    assert_same_object_changes_ignoring_version_and_digest(
+        dryrun_response.object_changes,
+        tx_response.object_changes.unwrap(),
     );
-    assert_eq!(dryrun_response.object_changes, object_changes.unwrap());
     Ok(())
 }
 
+fn assert_same_object_changes_ignoring_version_and_digest(
+    expected: Vec<ObjectChange>,
+    actual: Vec<ObjectChange>,
+) {
+    fn collect_changes_mask_version_and_digest(
+        changes: Vec<ObjectChange>,
+    ) -> BTreeMap<ObjectID, ObjectChange> {
+        changes
+            .into_iter()
+            .map(|mut change| {
+                let object_id = change.object_id();
+                // ignore the version and digest for comparison
+                change.mask_for_test(SequenceNumber::MAX, ObjectDigest::MAX);
+                (object_id, change)
+            })
+            .collect()
+    }
+    let expected = collect_changes_mask_version_and_digest(expected);
+    let actual = collect_changes_mask_version_and_digest(actual);
+    assert!(expected.keys().all(|id| actual.contains_key(id)));
+    assert!(actual.keys().all(|id| expected.contains_key(id)));
+    for (id, exp) in &expected {
+        let act = actual.get(id).unwrap();
+        assert_eq!(act, exp);
+    }
+}
+
 #[sim_test]
 async fn test_publish() -> Result<(), anyhow::Error> {
     let cluster = TestClusterBuilder::new().build().await?;

crates/sui-types/src/base_types.rs

@@ -759,11 +759,11 @@ impl SequenceNumber {
     pub const MIN: SequenceNumber = SequenceNumber(u64::MIN);
     pub const MAX: SequenceNumber = SequenceNumber(0x7fff_ffff_ffff_ffff);
 
-    pub fn new() -> Self {
+    pub const fn new() -> Self {
         SequenceNumber(0)
     }
 
-    pub fn value(&self) -> u64 {
+    pub const fn value(&self) -> u64 {
         self.0
     }
 

crates/sui-types/src/messages.rs

@@ -3177,6 +3177,10 @@ impl InputObjects {
         SequenceNumber::lamport_increment(input_versions)
     }
 
+    pub fn into_objects(self) -> Vec<(InputObjectKind, Object)> {
+        self.objects
+    }
+
     pub fn into_object_map(self) -> BTreeMap<ObjectID, Object> {
         self.objects
             .into_iter()

crates/sui-types/src/temporary_store.rs

@@ -176,6 +176,36 @@ impl<S> TemporaryStore<S> {
         }
     }
 
+    /// WARNING! Should only be used for dry run and dev inspect!
+    /// In dry run and dev inspect, you might load a dynamic field that is actually too new for
+    /// the transaction. Ideally, we would want to load the "correct" dynamic fields, but as that
+    /// is not easily determined, we instead set the lamport version MAX, which is a valid lamport
+    /// version for any object used in the transaction (preventing internal assertions or
+    /// invariant violations from being triggered)
+    pub fn new_for_mock_transaction(
+        store: S,
+        input_objects: InputObjects,
+        tx_digest: TransactionDigest,
+        protocol_config: &ProtocolConfig,
+    ) -> Self {
+        let mutable_inputs = input_objects.mutable_inputs();
+        let lamport_timestamp = SequenceNumber::MAX;
+        let objects = input_objects.into_object_map();
+        Self {
+            store,
+            tx_digest,
+            input_objects: objects,
+            lamport_timestamp,
+            mutable_input_refs: mutable_inputs,
+            written: BTreeMap::new(),
+            deleted: BTreeMap::new(),
+            events: Vec::new(),
+            gas_charged: None,
+            storage_rebate_rate: protocol_config.storage_rebate_rate(),
+            protocol_config: protocol_config.clone(),
+        }
+    }
+
     // Helpers to access private fields
     pub fn objects(&self) -> &BTreeMap<ObjectID, Object> {
         &self.input_objects