Merge branch 'develop' of https://github.com/stacks-network/stacks-core into chore/merge-develop-to-aac-client-breaking

Author: jferrant

CHANGELOG.md

@@ -10,6 +10,7 @@ and this project adheres to the versioning scheme outlined in the [README.md](RE
 ### Changed
 
 - Renamed Clarity 4's new `block-time` to `stacks-block-time`
+- Improve cost-tracking for type-checking function arguments in epoch 3.3 (see [#6425](https://github.com/stacks-network/stacks-core/issues/6425))
 
 ## [3.2.0.0.2]
 

clarity/src/vm/callables.rs

@@ -146,12 +146,18 @@ impl DefinedFunction {
             self.arguments.len(),
         )?;
 
-        for arg_type in self.arg_types.iter() {
-            runtime_cost(
-                ClarityCostFunction::InnerTypeCheckCost,
-                env,
-                arg_type.size()?,
-            )?;
+        if env.epoch().uses_arg_size_for_cost() {
+            for arg in args.iter() {
+                runtime_cost(ClarityCostFunction::InnerTypeCheckCost, env, arg.size()?)?;
+            }
+        } else {
+            for arg_type in self.arg_types.iter() {
+                runtime_cost(
+                    ClarityCostFunction::InnerTypeCheckCost,
+                    env,
+                    arg_type.size()?,
+                )?;
+            }
         }
 
         let mut context = LocalContext::new();

stacks-common/src/types/mod.rs

@@ -789,6 +789,27 @@ impl StacksEpochId {
             StacksEpochId::Epoch33 => true,
         }
     }
+
+    /// Before Epoch 3.3, the cost for arguments to functions was based on the
+    /// parameter type, not the actual size of the argument passed in. This
+    /// resulted in over-charging for arguments smaller than the maximum size
+    /// permitted for the parameter.
+    pub fn uses_arg_size_for_cost(&self) -> bool {
+        match self {
+            StacksEpochId::Epoch10
+            | StacksEpochId::Epoch20
+            | StacksEpochId::Epoch2_05
+            | StacksEpochId::Epoch21
+            | StacksEpochId::Epoch22
+            | StacksEpochId::Epoch23
+            | StacksEpochId::Epoch24
+            | StacksEpochId::Epoch25
+            | StacksEpochId::Epoch30
+            | StacksEpochId::Epoch31
+            | StacksEpochId::Epoch32 => false,
+            StacksEpochId::Epoch33 => true,
+        }
+    }
 }
 
 impl std::fmt::Display for StacksEpochId {

stackslib/src/chainstate/tests/consensus.rs

@@ -12,8 +12,8 @@
 //
 // You should have received a copy of the GNU General Public License
 // along with this program.  If not, see <http://www.gnu.org/licenses/>.
-use std::cell::LazyCell;
 use std::collections::{BTreeSet, HashMap};
+use std::sync::LazyLock;
 
 use clarity::boot_util::boot_code_addr;
 use clarity::codec::StacksMessageCodec;
@@ -62,7 +62,7 @@ pub const SK_2: &str = "4ce9a8f7539ea93753a36405b16e8b57e15a552430410709c2b6d65d
 pub const SK_3: &str = "cb95ddd0fe18ec57f4f3533b95ae564b3f1ae063dbf75b46334bd86245aef78501";
 
 /// The private key for the faucet account.
-pub const FAUCET_PRIV_KEY: LazyCell<StacksPrivateKey> = LazyCell::new(|| {
+pub static FAUCET_PRIV_KEY: LazyLock<StacksPrivateKey> = LazyLock::new(|| {
     StacksPrivateKey::from_hex("510f96a8efd0b11e211733c1ac5e3fa6f3d3fcdd62869e376c47decb3e14fea101")
         .expect("Failed to parse private key")
 });
@@ -709,6 +709,8 @@ pub struct ExpectedTransactionOutput {
 pub struct ExpectedBlockOutput {
     /// The expected block marf
     pub marf_hash: TrieHash,
+    /// The epoch in which the test block was expected to be evaluated
+    pub evaluated_epoch: StacksEpochId,
     /// The expected outputs for each transaction, in input order.
     pub transactions: Vec<ExpectedTransactionOutput>,
     /// The total execution cost of the block.
@@ -735,25 +737,25 @@ impl ExpectedResult {
             Ok(epoch_receipt) => {
                 let transactions: Vec<ExpectedTransactionOutput> = epoch_receipt
                     .tx_receipts
-                    .iter()
+                    .into_iter()
                     .map(|r| {
-                        let tx = match &r.transaction {
-                            TransactionOrigin::Stacks(tx) => Some(tx.payload.clone()),
+                        let tx = match r.transaction {
+                            TransactionOrigin::Stacks(tx) => Some(tx.payload),
                             TransactionOrigin::Burn(..) => None,
                         };
                         ExpectedTransactionOutput {
                             tx,
-                            return_type: r.result.clone(),
-                            cost: r.execution_cost.clone(),
-                            vm_error: r.vm_error.clone(),
+                            return_type: r.result,
+                            cost: r.execution_cost,
+                            vm_error: r.vm_error,
                         }
                     })
                     .collect();
-                let total_block_cost = epoch_receipt.anchored_block_cost.clone();
                 ExpectedResult::Success(ExpectedBlockOutput {
                     marf_hash,
+                    evaluated_epoch: epoch_receipt.evaluated_epoch,
                     transactions,
-                    total_block_cost,
+                    total_block_cost: epoch_receipt.anchored_block_cost,
                 })
             }
             Err(e) => ExpectedResult::Failure(e.to_string()),
@@ -873,7 +875,7 @@ impl ConsensusTest<'_> {
             "--------- Processed block: {sig_hash} ---------";
             "block" => ?nakamoto_block
         );
-        let remapped_result = res.map(|receipt| receipt.unwrap()).into();
+        let remapped_result = res.map(|receipt| receipt.unwrap());
         // Restore chainstate for the next block
         self.chain.sortdb = Some(sortdb);
         self.chain.stacks_node = Some(stacks_node);
@@ -1009,14 +1011,14 @@ impl ConsensusTest<'_> {
     fn compute_block_marf_root_hash(
         &mut self,
         block_time: u64,
-        block_txs: &Vec<StacksTransaction>,
+        block_txs: &[StacksTransaction],
     ) -> Result<TrieHash, String> {
         let node = self.chain.stacks_node.as_mut().unwrap();
         let sortdb = self.chain.sortdb.as_ref().unwrap();
         let burndb_conn = sortdb.index_handle_at_tip();
         let chainstate = &mut node.chainstate;
 
-        let chain_tip = NakamotoChainState::get_canonical_block_header(chainstate.db(), &sortdb)
+        let chain_tip = NakamotoChainState::get_canonical_block_header(chainstate.db(), sortdb)
             .unwrap()
             .unwrap();
 
@@ -1039,7 +1041,7 @@ impl ConsensusTest<'_> {
             chain_tip.burn_header_height,
         );
         clarity_tx.rollback_block();
-        return result;
+        result
     }
 
     /// This is where the real MARF computation happens.
@@ -1049,7 +1051,7 @@ impl ConsensusTest<'_> {
     fn inner_compute_block_marf_root_hash(
         clarity_tx: &mut ClarityTx,
         block_time: u64,
-        block_txs: &Vec<StacksTransaction>,
+        block_txs: &[StacksTransaction],
         burn_header_height: u32,
     ) -> Result<TrieHash, String> {
         clarity_tx

stackslib/src/chainstate/tests/snapshots/blockstack_lib__chainstate__tests__consensus__append_stx_transfers_success.snap

@@ -5,6 +5,7 @@ expression: result
 [
   Success(ExpectedBlockOutput(
     marf_hash: "95999ab12ae2162f2dd25c4a7f7807017b3b5d20f28a53248e37c9864f923718",
+    evaluated_epoch: Epoch32,
     transactions: [
       ExpectedTransactionOutput(
         tx: "TokenTransfer(from: ST000000000000000000002AMW42H, amount: 1000, memo: 00000000000000000000000000000000000000000000000000000000000000000000)",
@@ -62,6 +63,7 @@ expression: result
   )),
   Success(ExpectedBlockOutput(
     marf_hash: "7e9b86e5b0ff545908784c1c674d1354228ce2884395ced13f7c9e4eaa7ecfd0",
+    evaluated_epoch: Epoch33,
     transactions: [
       ExpectedTransactionOutput(
         tx: "TokenTransfer(from: ST000000000000000000002AMW42H, amount: 1000, memo: 00000000000000000000000000000000000000000000000000000000000000000000)",