Merge pull request #5912 from csgui/fix/clarity-wasm-context-rollback

Ensure context rollback in an error situation

Author: obycode

.github/workflows/github-release.yml

@@ -142,3 +142,21 @@ jobs:
           DOCKERHUB_USERNAME: ${{ secrets.DOCKERHUB_USERNAME }}
           DOCKERHUB_PASSWORD: ${{ secrets.DOCKERHUB_PASSWORD }}
           dist: ${{ matrix.dist }}
+
+  ## Create the downstream PR for the release branch to master,develop
+  create-pr:
+    if: |
+      inputs.node_tag != '' ||
+      inputs.signer_tag != ''
+    name: Create Downstream PR (${{ github.ref_name }})
+    runs-on: ubuntu-latest
+    needs:
+      - build-binaries
+      - create-release
+      - docker-image
+    steps:
+      - name: Open Downstream PR
+        id: create-pr
+        uses: stacks-network/actions/stacks-core/release/downstream-pr@main
+        with:
+          token: ${{ secrets.GH_TOKEN }}

.rustfmt.toml

@@ -0,0 +1,2 @@
+group_imports = "StdExternalCrate"
+imports_granularity = "Module"

.vscode/settings.json

@@ -1,7 +1,7 @@
 {
-    "lldb.adapterType": "native",
-    "lldb.launch.sourceLanguages": ["rust"],
-    "rust-analyzer.runnables.extraEnv": {
-        "BITCOIND_TEST": "1"
-    }
+  "lldb.launch.sourceLanguages": ["rust"],
+  "rust-analyzer.runnables.extraEnv": {
+    "BITCOIND_TEST": "1"
+  },
+  "rust-analyzer.rustfmt.extraArgs": ["+nightly"]
 }

CHANGELOG.md

@@ -7,9 +7,13 @@ and this project adheres to the versioning scheme outlined in the [README.md](RE
 
 ## [Unreleased]
 
+### Added"
+- Add fee information to transaction log ending with "success" or "skipped", while building a new block  
+
 ### Changed
 
 - When a miner times out waiting for signatures, it will re-propose the same block instead of building a new block ([#5877](https://github.com/stacks-network/stacks-core/pull/5877))
+- Improve tenure downloader trace verbosity applying proper logging level depending on the tenure state ("debug" if unconfirmed, "info" otherwise) ([#5871](https://github.com/stacks-network/stacks-core/issues/5871))
 
 ## [3.1.0.0.7]
 

Cargo.lock

@@ -428,11 +428,28 @@ pub fn initialize_contract(
         results.push(placeholder_for_type(result_ty));
     }
 
-    top_level
-        .call(&mut store, &[], results.as_mut_slice())
-        .map_err(|e| {
-            error_mapping::resolve_error(e, instance, &mut store, &epoch, &clarity_version)
-        })?;
+    let top_level_result = top_level.call(&mut store, &[], results.as_mut_slice());
+    match top_level_result {
+        Ok(_) => {}
+        Err(e) => {
+            // Before propagating the error, attempt to roll back the function context.
+            // If the rollback fails, immediately return a rollback-specific error.
+            if store.data_mut().global_context.roll_back().is_err() {
+                return Err(Error::Wasm(WasmError::Expect(
+                    "Expected entry to rollback".into(),
+                )));
+            }
+
+            // Rollback succeeded, so resolve and return the original runtime error.
+            return Err(error_mapping::resolve_error(
+                e,
+                instance,
+                &mut store,
+                &epoch,
+                &clarity_version,
+            ));
+        }
+    }
 
     // Save the compiled Wasm module into the contract context
     store.data_mut().contract_context_mut()?.set_wasm_module(

clarity/src/vm/clarity_wasm.rs

@@ -42,11 +42,11 @@ const FIRST_CLASS_TOKENS: &str = "(define-fungible-token stackaroos)
          (define-public (mint-after (block-to-release uint))
            (if (>= block-height block-to-release)
                (faucet)
-               (err \"must be in the future\")))
+               (err u100)))
          (define-public (burn (amount uint) (p principal))
            (ft-burn? stackaroos amount p))
-         (begin (ft-mint? stackaroos u10000 'SZ2J6ZY48GV1EZ5V2V5RB9MP66SW86PYKKQ9H6DPR)
-                (ft-mint? stackaroos u200 'SM2J6ZY48GV1EZ5V2V5RB9MP66SW86PYKKQVX8X0G)
+         (begin (is-ok (ft-mint? stackaroos u10000 'SZ2J6ZY48GV1EZ5V2V5RB9MP66SW86PYKKQ9H6DPR))
+                (is-ok (ft-mint? stackaroos u200 'SM2J6ZY48GV1EZ5V2V5RB9MP66SW86PYKKQVX8X0G))
                 (ft-mint? stackaroos u4 .tokens))";
 
 const ASSET_NAMES: &str =
@@ -80,15 +80,15 @@ const ASSET_NAMES: &str =
            (nft-burn? names name p))
          (define-public (try-bad-transfers)
            (begin
-             (contract-call? .tokens my-token-transfer burn-address u50000)
-             (contract-call? .tokens my-token-transfer burn-address u1000)
-             (contract-call? .tokens my-token-transfer burn-address u1)
+             (is-ok (contract-call? .tokens my-token-transfer burn-address u50000))
+             (is-ok (contract-call? .tokens my-token-transfer burn-address u1000))
+             (is-ok (contract-call? .tokens my-token-transfer burn-address u1))
              (err u0)))
          (define-public (try-bad-transfers-but-ok)
            (begin
-             (contract-call? .tokens my-token-transfer burn-address u50000)
-             (contract-call? .tokens my-token-transfer burn-address u1000)
-             (contract-call? .tokens my-token-transfer burn-address u1)
+             (is-ok (contract-call? .tokens my-token-transfer burn-address u50000))
+             (is-ok (contract-call? .tokens my-token-transfer burn-address u1000))
+             (is-ok (contract-call? .tokens my-token-transfer burn-address u1))
              (ok 0)))
          (define-public (transfer (name int) (recipient principal))
            (let ((transfer-name-result (nft-transfer? names name tx-sender recipient))
@@ -97,7 +97,7 @@ const ASSET_NAMES: &str =
              (begin (unwrap! transfer-name-result transfer-name-result)
                     (unwrap! token-to-contract-result token-to-contract-result)
                     (unwrap! contract-to-burn-result contract-to-burn-result)
-                    (ok 0))))
+                    (ok true))))
          (define-public (register
                         (recipient-principal principal)
                         (name int)

clarity/src/vm/tests/assets.rs

@@ -51,7 +51,7 @@ use clarity::util::hash::Sha256Sum;
 use clarity::util::retry::BoundReader;
 use clarity::util::secp256k1::MessageSignature;
 use clarity::vm::types::serialization::SerializationError;
-use clarity::vm::types::{QualifiedContractIdentifier, TupleData};
+use clarity::vm::types::{QualifiedContractIdentifier, ResponseData, TupleData};
 use clarity::vm::Value;
 use hashbrown::{HashMap, HashSet};
 use serde::{Deserialize, Serialize};
@@ -875,11 +875,11 @@ impl BlockResponse {
         }
     }
 
-    /// The signer signature hash for the block response
-    pub fn signer_signature_hash(&self) -> Sha512Trunc256Sum {
+    /// Get the block response data from the block response
+    pub fn get_response_data(&self) -> &BlockResponseData {
         match self {
-            BlockResponse::Accepted(accepted) => accepted.signer_signature_hash,
-            BlockResponse::Rejected(rejection) => rejection.signer_signature_hash,
+            BlockResponse::Accepted(accepted) => &accepted.response_data,
+            BlockResponse::Rejected(rejection) => &rejection.response_data,
         }
     }
 

libsigner/src/v0/messages.rs

@@ -7,9 +7,13 @@ and this project adheres to the versioning scheme outlined in the [README.md](RE
 
 ## [Unreleased]
 
+### Changed
+
+- For some rejection reasons, a signer will reconsider a block proposal that it previously rejected ([#5880](https://github.com/stacks-network/stacks-core/pull/5880))
+
 ## [3.1.0.0.7.0]
 
-## Changed
+### Changed
 
 - Add new reject codes to the signer response for better visibility into why a block was rejected.
 - When allowing a reorg within the `reorg_attempts_activity_timeout_ms`, the signer will now watch the responses from other signers and if >30% of them reject this reorg attempt, then the signer will mark the miner as invalid, reject further attempts to reorg and allow the previous miner to extend their tenure.
@@ -20,7 +24,7 @@ and this project adheres to the versioning scheme outlined in the [README.md](RE
 
 ## [3.1.0.0.6.0]
 
-## Added
+### Added
 
 - Introduced the `reorg_attempts_activity_timeout_ms` configuration option for signers which is used to determine the length of time after the last block of a tenure is confirmed that an incoming miner's attempts to reorg it are considered valid miner activity.
 - Add signer configuration option `tenure_idle_timeout_buffer_secs` to specify the number of seconds of buffer the signer will add to its tenure extend time that it sends to miners. The idea is to allow for some clock skew between the miner and signers, preventing the case where the miner attempts to tenure extend too early.

stacks-signer/CHANGELOG.md

@@ -167,6 +167,8 @@ pub struct BlockInfo {
     pub validation_time_ms: Option<u64>,
     /// Extra data specific to v0, v1, etc.
     pub ext: ExtraBlockInfo,
+    /// If this signer rejected this block, what was the reason
+    pub reject_reason: Option<RejectReason>,
 }
 
 impl From<BlockProposal> for BlockInfo {
@@ -184,6 +186,7 @@ impl From<BlockProposal> for BlockInfo {
             ext: ExtraBlockInfo::default(),
             state: BlockState::Unprocessed,
             validation_time_ms: None,
+            reject_reason: None,
         }
     }
 }
@@ -2193,4 +2196,77 @@ mod tests {
             .unwrap()
             .is_none());
     }
+
+    /// BlockInfo without the `reject_reason` field for backwards compatibility testing
+    #[derive(Serialize, Deserialize, Debug, PartialEq)]
+    pub struct BlockInfoPrev {
+        /// The block we are considering
+        pub block: NakamotoBlock,
+        /// The burn block height at which the block was proposed
+        pub burn_block_height: u64,
+        /// The reward cycle the block belongs to
+        pub reward_cycle: u64,
+        /// Our vote on the block if we have one yet
+        pub vote: Option<NakamotoBlockVote>,
+        /// Whether the block contents are valid
+        pub valid: Option<bool>,
+        /// Whether this block is already being signed over
+        pub signed_over: bool,
+        /// Time at which the proposal was received by this signer (epoch time in seconds)
+        pub proposed_time: u64,
+        /// Time at which the proposal was signed by this signer (epoch time in seconds)
+        pub signed_self: Option<u64>,
+        /// Time at which the proposal was signed by a threshold in the signer set (epoch time in seconds)
+        pub signed_group: Option<u64>,
+        /// The block state relative to the signer's view of the stacks blockchain
+        pub state: BlockState,
+        /// Consumed processing time in milliseconds to validate this block
+        pub validation_time_ms: Option<u64>,
+        /// Extra data specific to v0, v1, etc.
+        pub ext: ExtraBlockInfo,
+    }
+
+    /// Verify that we can deserialize the old BlockInfo struct into the new version
+    #[test]
+    fn deserialize_old_block_info() {
+        let block_info_prev = BlockInfoPrev {
+            block: NakamotoBlock {
+                header: NakamotoBlockHeader::genesis(),
+                txs: vec![],
+            },
+            burn_block_height: 2,
+            reward_cycle: 3,
+            vote: None,
+            valid: None,
+            signed_over: true,
+            proposed_time: 4,
+            signed_self: None,
+            signed_group: None,
+            state: BlockState::Unprocessed,
+            validation_time_ms: Some(5),
+            ext: ExtraBlockInfo::default(),
+        };
+
+        let block_info: BlockInfo =
+            serde_json::from_value(serde_json::to_value(&block_info_prev).unwrap()).unwrap();
+        assert_eq!(block_info.block, block_info_prev.block);
+        assert_eq!(
+            block_info.burn_block_height,
+            block_info_prev.burn_block_height
+        );
+        assert_eq!(block_info.reward_cycle, block_info_prev.reward_cycle);
+        assert_eq!(block_info.vote, block_info_prev.vote);
+        assert_eq!(block_info.valid, block_info_prev.valid);
+        assert_eq!(block_info.signed_over, block_info_prev.signed_over);
+        assert_eq!(block_info.proposed_time, block_info_prev.proposed_time);
+        assert_eq!(block_info.signed_self, block_info_prev.signed_self);
+        assert_eq!(block_info.signed_group, block_info_prev.signed_group);
+        assert_eq!(block_info.state, block_info_prev.state);
+        assert_eq!(
+            block_info.validation_time_ms,
+            block_info_prev.validation_time_ms
+        );
+        assert_eq!(block_info.ext, block_info_prev.ext);
+        assert!(block_info.reject_reason.is_none());
+    }
 }