Test fixes

Author: mystenmark

crates/sui-core/src/authority.rs

@@ -2,6 +2,7 @@
 // Copyright (c) Mysten Labs, Inc.
 // SPDX-License-Identifier: Apache-2.0
 
+use crate::accumulators::AccumulatorSettlementTxBuilder;
 use crate::accumulators::balance_read::AccountBalanceRead;
 use crate::checkpoints::CheckpointBuilderError;
 use crate::checkpoints::CheckpointBuilderResult;
@@ -1288,6 +1289,8 @@ impl AuthorityState {
             return Err(SuiErrorKind::ValidatorHaltedAtEpochEnd.into());
         }
 
+        transaction.validity_check(&epoch_store.tx_validity_check_context())?;
+
         let checked_input_objects =
             self.handle_transaction_deny_checks(transaction, epoch_store)?;
 
@@ -3909,6 +3912,66 @@ impl AuthorityState {
         Ok(new_epoch_store)
     }
 
+    pub async fn settle_transactions_for_testing(
+        &self,
+        ckpt_seq: CheckpointSequenceNumber,
+        effects: &[TransactionEffects],
+    ) {
+        let builder = AccumulatorSettlementTxBuilder::new(
+            Some(self.get_transaction_cache_reader().as_ref()),
+            effects,
+            ckpt_seq,
+            0,
+        );
+        let epoch_store = self.epoch_store_for_testing();
+        let epoch = epoch_store.epoch();
+        let (settlements, barrier) = builder.build_tx(
+            epoch_store.protocol_config(),
+            epoch,
+            epoch_store
+                .epoch_start_config()
+                .accumulator_root_obj_initial_shared_version()
+                .unwrap(),
+            ckpt_seq,
+            ckpt_seq,
+        );
+
+        let mut settlements: Vec<_> = settlements
+            .into_iter()
+            .map(|tx| {
+                VerifiedExecutableTransaction::new_system(
+                    VerifiedTransaction::new_system_transaction(tx),
+                    epoch,
+                )
+            })
+            .collect();
+        let barrier = VerifiedExecutableTransaction::new_system(
+            VerifiedTransaction::new_system_transaction(barrier),
+            epoch,
+        );
+
+        settlements.push(barrier);
+
+        let assigned_versions = epoch_store
+            .assign_shared_object_versions_for_tests(
+                self.get_object_cache_reader().as_ref(),
+                &settlements,
+            )
+            .unwrap();
+
+        let version_map = assigned_versions.into_map();
+
+        for tx in settlements {
+            let env = ExecutionEnv::new()
+                .with_assigned_versions(version_map.get(&tx.key()).unwrap().clone());
+            let (effects, _) = self
+                .try_execute_immediately(&tx, env, &epoch_store)
+                .await
+                .unwrap();
+            assert!(effects.status().is_ok());
+        }
+    }
+
     /// Advance the epoch store to the next epoch for testing only.
     /// This only manually sets all the places where we have the epoch number.
     /// It doesn't properly reconfigure the node, hence should be only used for testing.

crates/sui-core/src/execution_scheduler/execution_scheduler_impl.rs

@@ -316,8 +316,7 @@ impl ExecutionScheduler {
         for (cert, env) in &certs {
             let tx_withdraws = cert
                 .transaction_data()
-                .process_funds_withdrawals_for_signing()
-                .expect("Balance withdraws should have already been checked");
+                .process_funds_withdrawals_for_execution();
             assert!(!tx_withdraws.is_empty());
             let accumulator_version = env
                 .assigned_versions

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

@@ -215,17 +215,43 @@ async fn test_regulated_coin_v2_types() {
 // using the funds withdrawal argument in programmable transactions.
 #[tokio::test]
 async fn test_regulated_coin_v2_funds_withdraw_deny() {
+    telemetry_subscribers::init_for_testing();
     let env = new_authority_and_publish("coin_deny_list_v2").await;
 
     let metadata = env.extract_v2_metadata().await;
     let regulated_coin_type = metadata.regulated_coin_type();
     let deny_list_object_init_version = env.get_latest_object_ref(&SUI_DENY_LIST_OBJECT_ID).await.1;
-    let env_gas_ref = env.get_latest_object_ref(&env.gas_object_id).await;
+    let mut env_gas_ref = env.get_latest_object_ref(&env.gas_object_id).await;
     let deny_cap_ref = env.get_latest_object_ref(&metadata.deny_cap_id).await;
 
     // Create a new account that will be denied for the regulated coin.
     let (denied_address, denied_keypair) = get_account_key_pair();
 
+    env.authority
+        .settle_transactions_for_testing(0, std::slice::from_ref(&env.publish_effects))
+        .await;
+
+    {
+        // Fund the denied address
+        let tx = TestTransactionBuilder::new(
+            env.sender,
+            env_gas_ref,
+            env.authority.reference_gas_price_for_testing().unwrap(),
+        )
+        .transfer_funds_to_address_balance(100_000_000, regulated_coin_type.clone(), denied_address)
+        .build_and_sign(&env.keypair);
+        let effects = send_and_confirm_transaction_(&env.authority, None, tx, true)
+            .await
+            .unwrap()
+            .1;
+        assert!(effects.status().is_ok(), "Funding should succeed");
+        env_gas_ref = effects.gas_object().0;
+
+        env.authority
+            .settle_transactions_for_testing(1, std::slice::from_ref(&effects))
+            .await;
+    }
+
     // Add the denied address to the regulated coin deny list.
     let add_tx = TestTransactionBuilder::new(
         env.sender,
@@ -392,6 +418,7 @@ async fn new_authority_and_publish(path: &str) -> TestEnv {
 
     let mut protocol_config =
         ProtocolConfig::get_for_version(ProtocolVersion::max(), Chain::Unknown);
+    protocol_config.enable_accumulators_for_testing();
     protocol_config
         .set_per_object_congestion_control_mode_for_testing(PerObjectCongestionControlMode::None);
 

crates/sui-core/src/unit_tests/data/coin_deny_list_v2/sources/regulated_coin.move

@@ -1,24 +1,27 @@
 // Copyright (c) Mysten Labs, Inc.
 // SPDX-License-Identifier: Apache-2.0
 
-module coin_deny_list_v2::regulated_coin {
-    use sui::coin;
+module coin_deny_list_v2::regulated_coin;
 
-    public struct REGULATED_COIN has drop {}
+use sui::coin;
 
-    fun init(otw: REGULATED_COIN, ctx: &mut TxContext) {
-        let (treasury_cap, deny_cap, metadata) = coin::create_regulated_currency_v2(
-            otw,
-            9,
-            b"RC",
-            b"REGULATED_COIN",
-            b"A new regulated coin",
-            option::none(),
-            true,
-            ctx
-        );
-        transfer::public_transfer(deny_cap, tx_context::sender(ctx));
-        transfer::public_freeze_object(treasury_cap);
-        transfer::public_freeze_object(metadata);
-    }
+public struct REGULATED_COIN has drop {}
+
+fun init(otw: REGULATED_COIN, ctx: &mut TxContext) {
+  let (mut treasury_cap, deny_cap, metadata) = coin::create_regulated_currency_v2(
+    otw,
+    9,
+    b"RC",
+    b"REGULATED_COIN",
+    b"A new regulated coin",
+    option::none(),
+    true,
+    ctx,
+  );
+  transfer::public_transfer(deny_cap, tx_context::sender(ctx));
+
+  let coin = treasury_cap.mint(100_000_000, ctx);
+  coin.send_funds(tx_context::sender(ctx));
+  transfer::public_freeze_object(treasury_cap);
+  transfer::public_freeze_object(metadata);
 }

crates/sui-e2e-tests/tests/address_balance_tests.rs

@@ -25,7 +25,7 @@ use sui_types::{
     supported_protocol_versions::SupportedProtocolVersions,
     transaction::{
         Argument, Command, FundsWithdrawalArg, GasData, Transaction, TransactionData,
-        TransactionDataV1, TransactionExpiration, TransactionKind,
+        TransactionDataAPI, TransactionDataV1, TransactionExpiration, TransactionKind,
     },
 };
 use test_cluster::TestClusterBuilder;
@@ -744,6 +744,26 @@ async fn test_address_balance_gas() {
 
     let chain_id = test_cluster.get_chain_identifier();
 
+    // check that a transaction with a zero gas budget is rejected
+    {
+        let mut tx = create_storage_test_transaction_address_balance(
+            sender,
+            gas_package_id,
+            rgp,
+            chain_id,
+            None,
+            0,
+        );
+        tx.gas_data_mut().budget = 0;
+        let signed_tx = test_cluster.sign_transaction(&tx).await;
+        let err = test_cluster
+            .wallet
+            .execute_transaction_may_fail(signed_tx)
+            .await
+            .unwrap_err();
+        assert!(err.to_string().contains("Gas budget: 0 is lower than min:"));
+    }
+
     let tx = create_storage_test_transaction_address_balance(
         sender,
         gas_package_id,
@@ -2275,7 +2295,7 @@ async fn test_multiple_deposits_merged_in_effects() {
     let rgp = test_cluster.get_reference_gas_price().await;
     let context = &mut test_cluster.wallet;
 
-    let (sender, gas) = get_sender_and_gas(context).await;
+    let (sender, gas) = get_sender_and_one_gas(context).await;
     let recipient = sender;
 
     let initial_deposit = make_send_to_account_tx(10000, recipient, sender, gas, rgp);

crates/sui-test-transaction-builder/src/lib.rs

@@ -14,6 +14,7 @@ use sui_types::SUI_RANDOMNESS_STATE_OBJECT_ID;
 use sui_types::base_types::{FullObjectRef, ObjectID, ObjectRef, SequenceNumber, SuiAddress};
 use sui_types::crypto::{AccountKeyPair, Signature, Signer, get_key_pair};
 use sui_types::digests::TransactionDigest;
+use sui_types::gas_coin::GAS;
 use sui_types::multisig::{BitmapUnit, MultiSig, MultiSigPublicKey};
 use sui_types::multisig_legacy::{MultiSigLegacy, MultiSigPublicKeyLegacy};
 use sui_types::object::Owner;
@@ -294,6 +295,31 @@ impl TestTransactionBuilder {
         self
     }
 
+    pub fn transfer_sui_to_address_balance(mut self, amount: u64, recipient: SuiAddress) -> Self {
+        self.test_data =
+            TestTransactionData::TransferFundsToAddressBalance(TransferFundsToAddressBalanceData {
+                amount,
+                type_arg: GAS::type_tag(),
+                recipient,
+            });
+        self
+    }
+
+    pub fn transfer_funds_to_address_balance(
+        mut self,
+        amount: u64,
+        type_arg: TypeTag,
+        recipient: SuiAddress,
+    ) -> Self {
+        self.test_data =
+            TestTransactionData::TransferFundsToAddressBalance(TransferFundsToAddressBalanceData {
+                amount,
+                type_arg,
+                recipient,
+            });
+        self
+    }
+
     pub fn split_coin(mut self, coin: ObjectRef, amounts: Vec<u64>) -> Self {
         self.test_data = TestTransactionData::SplitCoin(SplitCoinData { coin, amounts });
         self
@@ -368,6 +394,19 @@ impl TestTransactionBuilder {
                     .unwrap_or(self.gas_price * TEST_ONLY_GAS_UNIT_FOR_TRANSFER),
                 self.gas_price,
             ),
+            TestTransactionData::TransferFundsToAddressBalance(data) => {
+                TransactionData::new_transfer_funds_to_address_balance(
+                    data.recipient,
+                    self.sender,
+                    data.amount,
+                    data.type_arg,
+                    self.gas_object,
+                    self.gas_budget
+                        .unwrap_or(self.gas_price * TEST_ONLY_GAS_UNIT_FOR_TRANSFER),
+                    self.gas_price,
+                )
+                .unwrap()
+            }
             TestTransactionData::SplitCoin(data) => TransactionData::new_split_coin(
                 self.sender,
                 data.coin,
@@ -474,6 +513,7 @@ enum TestTransactionData {
     Move(MoveData),
     Transfer(TransferData),
     TransferSui(TransferSuiData),
+    TransferFundsToAddressBalance(TransferFundsToAddressBalanceData),
     SplitCoin(SplitCoinData),
     Publish(PublishData),
     Programmable(ProgrammableTransaction),
@@ -507,6 +547,12 @@ struct TransferSuiData {
     recipient: SuiAddress,
 }
 
+struct TransferFundsToAddressBalanceData {
+    amount: u64,
+    type_arg: TypeTag,
+    recipient: SuiAddress,
+}
+
 struct SplitCoinData {
     coin: ObjectRef,
     amounts: Vec<u64>,

crates/sui-types/src/balance.rs

@@ -19,6 +19,8 @@ pub const BALANCE_STRUCT_NAME: &IdentStr = ident_str!("Balance");
 pub const BALANCE_CREATE_REWARDS_FUNCTION_NAME: &IdentStr = ident_str!("create_staking_rewards");
 pub const BALANCE_DESTROY_REBATES_FUNCTION_NAME: &IdentStr = ident_str!("destroy_storage_rebates");
 
+pub const BALANCE_REDEEM_FUNDS_FUNCTION_NAME: &IdentStr = ident_str!("redeem_funds");
+pub const BALANCE_SEND_FUNDS_FUNCTION_NAME: &IdentStr = ident_str!("send_funds");
 #[serde_as]
 #[derive(Debug, Serialize, Deserialize, Clone, Eq, PartialEq, JsonSchema)]
 pub struct Supply {

crates/sui-types/src/gas.rs

@@ -11,7 +11,7 @@ use crate::{base_types::ObjectID, gas_model::gas_v2::PerObjectStorage};
 pub mod checked {
 
     use crate::gas::GasUsageReport;
-    use crate::gas_model::gas_predicates::gas_price_too_high;
+    use crate::gas_model::gas_predicates::check_for_gas_price_too_high;
     use crate::{
         ObjectID,
         effects::{TransactionEffects, TransactionEffectsAPI},
@@ -83,7 +83,8 @@ pub mod checked {
                 }
                 .into());
             }
-            if gas_price_too_high(config.gas_model_version()) && gas_price >= config.max_gas_price()
+            if check_for_gas_price_too_high(config.gas_model_version())
+                && gas_price >= config.max_gas_price()
             {
                 return Err(UserInputError::GasPriceTooHigh {
                     max_gas_price: config.max_gas_price(),

crates/sui-types/src/gas_model/gas_predicates.rs

@@ -22,7 +22,7 @@ pub fn dont_charge_budget_on_storage_oog(gas_model_version: u64) -> bool {
 }
 
 /// If true, enable the check for gas price too high
-pub fn gas_price_too_high(gas_model_version: u64) -> bool {
+pub fn check_for_gas_price_too_high(gas_model_version: u64) -> bool {
     gas_model_version >= 4
 }