Merge pull request #14 from multiversx/tests

tests + fixes

common/tx-batch-module/src/lib.rs

@@ -4,13 +4,10 @@ multiversx_sc::imports!();
 multiversx_sc::derive_imports!();
 
 pub use batch_status::BatchStatus;
-use transaction::{
-    BatchId, GasLimit, Transaction, TxBatchSplitInFields, TxNonce, MIN_BLOCKS_FOR_FINALITY,
-};
+use transaction::{BatchId, Transaction, TxBatchSplitInFields, TxNonce, MIN_BLOCKS_FOR_FINALITY};
 use tx_batch_mapper::TxBatchMapper;
 
 pub const FIRST_BATCH_ID: BatchId = 1;
-pub const MAX_GAS_LIMIT_PER_BATCH: GasLimit = 500_000_000;
 
 pub mod batch_status;
 pub mod tx_batch_mapper;
@@ -121,30 +118,16 @@ pub trait TxBatchModule {
     // private
 
     fn add_to_batch(&self, transaction: Transaction<Self::Api>) -> BatchId {
-        let default_gas_limit = self.sovereign_tx_gas_limit().get();
         let first_batch_id = self.first_batch_id().get();
         let last_batch_id = self.last_batch_id().get();
         let mut last_batch = self.pending_batches(last_batch_id);
 
-        let gas_cost = match &transaction.opt_transfer_data {
-            Some(transfer_data) => transfer_data.gas_limit,
-            None => transaction.tokens.len() as u64 * default_gas_limit,
-        };
-
-        let gas_cost_mapper = self.total_gas_cost(last_batch_id);
-        let last_batch_total_gas_cost = gas_cost_mapper.get();
-        let new_total_gas_cost = last_batch_total_gas_cost + gas_cost;
-
-        if self.is_batch_full(&last_batch, last_batch_id, first_batch_id)
-            || new_total_gas_cost > MAX_GAS_LIMIT_PER_BATCH
-        {
+        if self.is_batch_full(&last_batch, last_batch_id, first_batch_id) {
             let (new_batch_id, _) = self.create_new_batch(transaction);
-            self.total_gas_cost(new_batch_id).set(gas_cost);
 
             new_batch_id
         } else {
             last_batch.push(transaction);
-            gas_cost_mapper.set(new_total_gas_cost);
 
             last_batch_id
         }
@@ -159,37 +142,21 @@ pub trait TxBatchModule {
             return ManagedVec::new();
         }
 
-        let default_gas_limit = self.sovereign_tx_gas_limit().get();
         let first_batch_id = self.first_batch_id().get();
         let mut last_batch_id = self.last_batch_id().get();
         let mut last_batch = self.pending_batches(last_batch_id);
         let mut batch_ids = ManagedVec::new();
-        let mut total_gas_cost = self.total_gas_cost(last_batch_id).get();
 
         for tx in transactions {
-            let gas_cost = match &tx.opt_transfer_data {
-                Some(transfer_data) => transfer_data.gas_limit,
-                None => tx.tokens.len() as u64 * default_gas_limit,
-            };
-
-            let new_total_gas_cost = total_gas_cost + gas_cost;
-            if self.is_batch_full(&last_batch, last_batch_id, first_batch_id)
-                || new_total_gas_cost > MAX_GAS_LIMIT_PER_BATCH
-            {
-                self.total_gas_cost(last_batch_id).set(total_gas_cost);
-                total_gas_cost = gas_cost;
-
+            if self.is_batch_full(&last_batch, last_batch_id, first_batch_id) {
                 (last_batch_id, last_batch) = self.create_new_batch(tx);
             } else {
-                total_gas_cost = new_total_gas_cost;
                 last_batch.push(tx);
             }
 
             batch_ids.push(last_batch_id);
         }
 
-        self.total_gas_cost(last_batch_id).set(total_gas_cost);
-
         batch_ids
     }
 
@@ -297,9 +264,6 @@ pub trait TxBatchModule {
     #[storage_mapper("pendingBatches")]
     fn pending_batches(&self, batch_id: BatchId) -> TxBatchMapper<Self::Api>;
 
-    #[storage_mapper("totalGasCost")]
-    fn total_gas_cost(&self, batch_id: BatchId) -> SingleValueMapper<GasLimit>;
-
     #[storage_mapper("lastTxNonce")]
     fn last_tx_nonce(&self) -> SingleValueMapper<TxNonce>;
 
@@ -310,8 +274,4 @@ pub trait TxBatchModule {
 
     #[storage_mapper("maxTxBatchBlockDuration")]
     fn max_tx_batch_block_duration(&self) -> SingleValueMapper<u64>;
-
-    #[view(getSovereignTxGasLimit)]
-    #[storage_mapper("sovereignTxGasLimit")]
-    fn sovereign_tx_gas_limit(&self) -> SingleValueMapper<GasLimit>;
 }

esdt-safe/Cargo.toml

@@ -32,3 +32,8 @@ version = "=0.43.5"
 
 [dev-dependencies.multiversx-sc-scenario]
 version = "=0.43.5"
+
+[dev-dependencies]
+num-bigint = "0.4.2"
+num-traits = "0.2"
+hex = "0.4"

esdt-safe/src/from_sovereign/refund.rs

@@ -4,12 +4,13 @@ multiversx_sc::imports!();
 
 const NFT_AMOUNT: u32 = 1;
 
-pub struct CheckMustRefundArgs<M: ManagedTypeApi> {
-    pub token: EsdtTokenPayment<M>,
+pub struct CheckMustRefundArgs<'a, M: ManagedTypeApi> {
+    pub token: &'a EsdtTokenPayment<M>,
     pub roles: EsdtLocalRoleFlags,
-    pub dest: ManagedAddress<M>,
+    pub dest: &'a ManagedAddress<M>,
     pub batch_id: BatchId,
     pub tx_nonce: TxNonce,
+    pub sc_address: &'a ManagedAddress<M>,
     pub sc_shard: u32,
 }
 
@@ -20,16 +21,23 @@ pub trait RefundModule:
     + max_bridged_amount_module::MaxBridgedAmountModule
 {
     fn check_must_refund(&self, args: CheckMustRefundArgs<Self::Api>) -> bool {
-        if args.token.token_nonce == 0 {
-            if !args.roles.has_role(&EsdtLocalRole::Mint) {
+        let token_balance = self.blockchain().get_esdt_balance(
+            args.sc_address,
+            &args.token.token_identifier,
+            args.token.token_nonce,
+        );
+        if token_balance < args.token.amount {
+            if args.token.token_nonce == 0 {
+                if !args.roles.has_role(&EsdtLocalRole::Mint) {
+                    self.transfer_failed_invalid_token(args.batch_id, args.tx_nonce);
+
+                    return true;
+                }
+            } else if !self.has_nft_roles(args.token, args.roles) {
                 self.transfer_failed_invalid_token(args.batch_id, args.tx_nonce);
 
                 return true;
             }
-        } else if !self.has_nft_roles(&args.token, args.roles) {
-            self.transfer_failed_invalid_token(args.batch_id, args.tx_nonce);
-
-            return true;
         }
 
         if self.is_above_max_amount(&args.token.token_identifier, &args.token.amount) {
@@ -38,11 +46,8 @@ pub trait RefundModule:
             return true;
         }
 
-        if self.is_account_same_shard_frozen(
-            args.sc_shard,
-            &args.dest,
-            &args.token.token_identifier,
-        ) {
+        if self.is_account_same_shard_frozen(args.sc_shard, args.dest, &args.token.token_identifier)
+        {
             self.transfer_failed_frozen_destination_account(args.batch_id, args.tx_nonce);
 
             return true;

esdt-safe/src/from_sovereign/transfer_tokens.rs

@@ -10,7 +10,7 @@ use crate::from_sovereign::refund::CheckMustRefundArgs;
 
 multiversx_sc::imports!();
 
-const CALLBACK_GAS: GasLimit = 1_000_000; // Increase if not enough
+const CALLBACK_GAS: GasLimit = 10_000_000; // Increase if not enough
 
 #[multiversx_sc::module]
 pub trait TransferTokensModule:
@@ -32,6 +32,9 @@ pub trait TransferTokensModule:
     ) {
         require!(self.not_paused(), "Cannot transfer while paused");
 
+        let next_batch_id = self.next_batch_id().get();
+        require!(batch_id == next_batch_id, "Unexpected batch ID");
+
         let mut successful_tx_list = ManagedVec::new();
         let mut all_tokens_to_send = ManagedVec::new();
         let mut refund_tx_list = ManagedVec::new();
@@ -51,11 +54,12 @@ pub trait TransferTokensModule:
                     .blockchain()
                     .get_esdt_local_roles(&token.token_identifier);
                 let must_refund_args = CheckMustRefundArgs {
-                    token: token.clone(),
+                    token: &token,
                     roles: token_roles,
-                    dest: sov_tx.to.clone(),
+                    dest: &sov_tx.to,
                     batch_id,
                     tx_nonce: sov_tx.nonce,
+                    sc_address: &own_sc_address,
                     sc_shard,
                 };
                 let must_refund = self.check_must_refund(must_refund_args);
@@ -86,6 +90,8 @@ pub trait TransferTokensModule:
         self.distribute_payments(batch_id, successful_tx_list, all_tokens_to_send);
 
         self.add_multiple_tx_to_batch(&refund_tx_list);
+
+        self.next_batch_id().set(batch_id + 1);
     }
 
     fn mint_tokens(
@@ -201,4 +207,7 @@ pub trait TransferTokensModule:
             }
         }
     }
+
+    #[storage_mapper("nextBatchId")]
+    fn next_batch_id(&self) -> SingleValueMapper<BatchId>;
 }

esdt-safe/src/lib.rs

@@ -3,7 +3,6 @@
 multiversx_sc::imports!();
 multiversx_sc::derive_imports!();
 
-use transaction::GasLimit;
 use tx_batch_module::FIRST_BATCH_ID;
 
 const DEFAULT_MAX_TX_BATCH_SIZE: usize = 10;
@@ -29,25 +28,16 @@ pub trait EsdtSafe:
     + multiversx_sc_modules::pause::PauseModule
     + multiversx_sc_modules::default_issue_callbacks::DefaultIssueCallbacksModule
 {
-    /// sovereign_tx_gas_limit - The gas limit that will be used for transactions on the Sovereign side.
-    /// In case of SC gas limits, this value is provided by the user
-    /// Will be used to compute the fees for the transfer
     #[init]
-    fn init(
-        &self,
-        sovereign_tx_gas_limit: GasLimit,
-        min_valid_signers: u32,
-        signers: MultiValueEncoded<ManagedAddress>,
-    ) {
-        self.sovereign_tx_gas_limit().set(sovereign_tx_gas_limit);
-
+    fn init(&self, min_valid_signers: u32, signers: MultiValueEncoded<ManagedAddress>) {
         self.max_tx_batch_size().set(DEFAULT_MAX_TX_BATCH_SIZE);
         self.max_tx_batch_block_duration()
             .set(DEFAULT_MAX_TX_BATCH_BLOCK_DURATION);
 
         // batch ID 0 is considered invalid
         self.first_batch_id().set(FIRST_BATCH_ID);
         self.last_batch_id().set(FIRST_BATCH_ID);
+        self.next_batch_id().set(FIRST_BATCH_ID);
 
         self.set_min_valid_signers(min_valid_signers);
         self.add_signers(signers);

esdt-safe/tests/bridge_setup/mod.rs

@@ -0,0 +1,76 @@
+use bls_signature::BLS_SIGNATURE_LEN;
+use esdt_safe::EsdtSafe;
+use multiversx_sc::types::{Address, MultiValueEncoded};
+use multiversx_sc_modules::pause::PauseModule;
+use multiversx_sc_scenario::{
+    managed_token_id, rust_biguint,
+    testing_framework::{BlockchainStateWrapper, ContractObjWrapper},
+    DebugApi,
+};
+use token_module::TokenModule;
+use tx_batch_module::TxBatchModule;
+
+multiversx_sc::derive_imports!();
+
+pub static FUNGIBLE_TOKEN_ID: &[u8] = b"FUNGTOKEN-123456";
+pub static NFT_TOKEN_ID: &[u8] = b"NFT-123456";
+pub const TOKEN_BALANCE: u64 = 1_000_000_000_000_000_000;
+pub static DUMMY_SIG: [u8; BLS_SIGNATURE_LEN] = [0; BLS_SIGNATURE_LEN];
+
+#[derive(TopEncode, TopDecode, PartialEq, Debug)]
+pub struct DummyAttributes {
+    pub dummy: u8,
+}
+
+pub struct BridgeSetup<BridgeBuilder>
+where
+    BridgeBuilder: 'static + Copy + Fn() -> esdt_safe::ContractObj<DebugApi>,
+{
+    pub b_mock: BlockchainStateWrapper,
+    pub owner: Address,
+    pub user: Address,
+    pub sov_dest_addr: Address,
+    pub bridge_wrapper: ContractObjWrapper<esdt_safe::ContractObj<DebugApi>, BridgeBuilder>,
+}
+
+impl<BridgeBuilder> BridgeSetup<BridgeBuilder>
+where
+    BridgeBuilder: 'static + Copy + Fn() -> esdt_safe::ContractObj<DebugApi>,
+{
+    pub fn new(bridge_builder: BridgeBuilder) -> Self {
+        let rust_zero = rust_biguint!(0);
+        let mut b_mock = BlockchainStateWrapper::new();
+        let owner = b_mock.create_user_account(&rust_zero);
+        let user = b_mock.create_user_account(&rust_zero);
+        let sov_dest_addr = b_mock.create_user_account(&rust_zero);
+        let bridge_wrapper =
+            b_mock.create_sc_account(&rust_zero, Some(&owner), bridge_builder, "bridge");
+
+        b_mock.set_esdt_balance(&user, FUNGIBLE_TOKEN_ID, &rust_biguint!(TOKEN_BALANCE));
+        b_mock.set_nft_balance(
+            &user,
+            NFT_TOKEN_ID,
+            1,
+            &rust_biguint!(TOKEN_BALANCE),
+            &DummyAttributes { dummy: 42 },
+        );
+
+        b_mock
+            .execute_tx(&owner, &bridge_wrapper, &rust_zero, |sc| {
+                sc.init(0, MultiValueEncoded::new());
+                sc.set_max_tx_batch_size(1);
+                sc.set_paused(false);
+                sc.add_token_to_whitelist(managed_token_id!(FUNGIBLE_TOKEN_ID));
+                sc.add_token_to_whitelist(managed_token_id!(NFT_TOKEN_ID));
+            })
+            .assert_ok();
+
+        BridgeSetup {
+            b_mock,
+            owner,
+            user,
+            sov_dest_addr,
+            bridge_wrapper,
+        }
+    }
+}