Merge pull request #2112 from blockstack/fix/2097

Fix: #2097, event dispatcher should not unpack response results

Author: Aaron Blankstein

src/chainstate/stacks/db/transactions.rs

@@ -3555,6 +3555,120 @@ pub mod test {
         conn.commit_block();
     }
 
+    #[test]
+    fn process_post_conditions_tokens_deny_2097() {
+        let privk_origin = StacksPrivateKey::from_hex(
+            "027682d2f7b05c3801fe4467883ab4cff0568b5e36412b5289e83ea5b519de8a01",
+        )
+        .unwrap();
+        let privk_recipient = StacksPrivateKey::from_hex(
+            "7e3af4db6af6b3c67e2c6c6d7d5983b519f4d9b3a6e00580ae96dcace3bde8bc01",
+        )
+        .unwrap();
+        let auth_origin = TransactionAuth::from_p2pkh(&privk_origin).unwrap();
+        let auth_recv = TransactionAuth::from_p2pkh(&privk_recipient).unwrap();
+        let addr_publisher = auth_origin.origin().address_testnet();
+        let addr_principal = addr_publisher.to_account_principal();
+
+        let contract = "
+(define-constant owner 'ST3X2W2SH9XQZRHHYJ21KWGTT1N6WX3D48K1NSTPE)
+(define-fungible-token connect-token)
+(begin (ft-mint? connect-token u100000000 owner))
+(define-public (transfer (recipient principal) (amount uint))
+  (ok (ft-transfer? connect-token amount tx-sender recipient)))
+"
+        .to_string();
+
+        let contract_name = ContractName::try_from("hello-world").unwrap();
+
+        let recv_addr = StacksAddress::from_public_keys(
+            C32_ADDRESS_VERSION_TESTNET_SINGLESIG,
+            &AddressHashMode::SerializeP2PKH,
+            1,
+            &vec![StacksPublicKey::from_private(&privk_recipient)],
+        )
+        .unwrap();
+        let recv_principal = recv_addr.to_account_principal();
+        let contract_id = QualifiedContractIdentifier::new(
+            StandardPrincipalData::from(addr_publisher.clone()),
+            contract_name.clone(),
+        );
+        let _contract_principal = PrincipalData::Contract(contract_id.clone());
+
+        let asset_info = AssetInfo {
+            contract_address: addr_publisher.clone(),
+            contract_name: contract_name.clone(),
+            asset_name: ClarityName::try_from("connect-token").unwrap(),
+        };
+
+        let mut tx_contract = StacksTransaction::new(
+            TransactionVersion::Testnet,
+            auth_origin.clone(),
+            TransactionPayload::new_smart_contract(&"hello-world".to_string(), &contract).unwrap(),
+        );
+
+        tx_contract.chain_id = 0x80000000;
+        tx_contract.set_fee_rate(0);
+
+        let mut signer = StacksTransactionSigner::new(&tx_contract);
+        signer.sign_origin(&privk_origin).unwrap();
+
+        let signed_contract_tx = signer.get_tx().unwrap();
+
+        let mut tx_contract_call = StacksTransaction::new(
+            TransactionVersion::Testnet,
+            auth_origin.clone(),
+            TransactionPayload::new_contract_call(
+                addr_publisher.clone(),
+                "hello-world",
+                "transfer",
+                vec![Value::Principal(recv_principal.clone()), Value::UInt(10)],
+            )
+            .unwrap(),
+        );
+
+        tx_contract_call.chain_id = 0x80000000;
+        tx_contract_call.set_fee_rate(0);
+        tx_contract_call.set_origin_nonce(1);
+
+        tx_contract_call.post_condition_mode = TransactionPostConditionMode::Deny;
+        tx_contract_call.add_post_condition(TransactionPostCondition::Fungible(
+            PostConditionPrincipal::Origin,
+            asset_info.clone(),
+            FungibleConditionCode::SentEq,
+            10,
+        ));
+
+        let mut signer = StacksTransactionSigner::new(&tx_contract_call);
+        signer.sign_origin(&privk_origin).unwrap();
+        let contract_call_tx = signer.get_tx().unwrap();
+
+        let mut chainstate = instantiate_chainstate(
+            false,
+            0x80000000,
+            "process-post-conditions-tokens-deny-2097",
+        );
+        let mut conn = chainstate.block_begin(
+            &NULL_BURN_STATE_DB,
+            &FIRST_BURNCHAIN_CONSENSUS_HASH,
+            &FIRST_STACKS_BLOCK_HASH,
+            &ConsensusHash([1u8; 20]),
+            &BlockHeaderHash([1u8; 32]),
+        );
+
+        // publish contract
+        let _ =
+            StacksChainState::process_transaction(&mut conn, &signed_contract_tx, false).unwrap();
+
+        let (_fee, receipt) =
+            StacksChainState::process_transaction(&mut conn, &contract_call_tx, false).unwrap();
+
+        assert_eq!(receipt.post_condition_aborted, true);
+        assert_eq!(receipt.result.to_string(), "(ok (err u1))");
+
+        conn.commit_block();
+    }
+
     fn make_account(principal: &PrincipalData, nonce: u64, balance: u128) -> StacksAccount {
         let mut stx_balance = STXBalance::zero();
         stx_balance.credit(balance, 0).unwrap();

testnet/stacks-node/src/event_dispatcher.rs

@@ -145,38 +145,30 @@ impl EventObserver {
     ) -> serde_json::Value {
         let tx = &receipt.transaction;
 
-        let (success, result) = match (receipt.post_condition_aborted, &receipt.result) {
+        let success = match (receipt.post_condition_aborted, &receipt.result) {
             (false, Value::Response(response_data)) => {
-                let status = if response_data.committed {
+                if response_data.committed {
                     STATUS_RESP_TRUE
                 } else {
                     STATUS_RESP_NOT_COMMITTED
-                };
-                (status, response_data.data.clone())
-            }
-            (true, Value::Response(response_data)) => {
-                (STATUS_RESP_POST_CONDITION, response_data.data.clone())
+                }
             }
+            (true, Value::Response(_)) => STATUS_RESP_POST_CONDITION,
             _ => unreachable!(), // Transaction results should always be a Value::Response type
         };
 
         let (txid, raw_tx) = match tx {
             TransactionOrigin::Burn(txid) => (txid.to_string(), "00".to_string()),
             TransactionOrigin::Stacks(ref tx) => {
                 let txid = tx.txid().to_string();
-                let mut bytes = vec![];
-                tx.consensus_serialize(&mut bytes).unwrap();
-                let formatted_bytes: Vec<String> =
-                    bytes.iter().map(|b| format!("{:02x}", b)).collect();
-                (txid, formatted_bytes.join(""))
+                let bytes = tx.serialize_to_vec();
+                (txid, bytes_to_hex(&bytes))
             }
         };
 
         let raw_result = {
-            let mut bytes = vec![];
-            result.consensus_serialize(&mut bytes).unwrap();
-            let formatted_bytes: Vec<String> = bytes.iter().map(|b| format!("{:02x}", b)).collect();
-            formatted_bytes
+            let bytes = receipt.result.serialize_to_vec();
+            bytes_to_hex(&bytes)
         };
         let contract_interface_json = {
             match &receipt.contract_analysis {
@@ -188,7 +180,7 @@ impl EventObserver {
             "txid": format!("0x{}", &txid),
             "tx_index": tx_index,
             "status": success,
-            "raw_result": format!("0x{}", raw_result.join("")),
+            "raw_result": format!("0x{}", &raw_result),
             "raw_tx": format!("0x{}", &raw_tx),
             "contract_abi": contract_interface_json,
         })

testnet/stacks-node/src/tests/neon_integrations.rs

@@ -6,16 +6,19 @@ use stacks::burnchains::{Address, Burnchain, PoxConstants};
 use stacks::chainstate::burn::ConsensusHash;
 use stacks::chainstate::stacks::{
     db::StacksChainState, StacksAddress, StacksBlock, StacksBlockHeader, StacksPrivateKey,
-    StacksPublicKey, StacksTransaction,
+    StacksPublicKey, StacksTransaction, TransactionPayload,
 };
 use stacks::core;
 use stacks::net::StacksMessageCodec;
+use stacks::util::hash::hex_bytes;
 use stacks::util::secp256k1::Secp256k1PublicKey;
 use stacks::vm::costs::ExecutionCost;
 use stacks::vm::execute;
 use stacks::vm::types::PrincipalData;
 use stacks::vm::Value;
 
+use stacks::vm::database::ClarityDeserializable;
+
 use super::bitcoin_regtest::BitcoinCoreController;
 use crate::{
     config::EventKeyType, config::EventObserverConfig, config::InitialBalance, neon,
@@ -772,6 +775,13 @@ fn pox_integration_test() {
 
     let (mut conf, miner_account) = neon_integration_test_conf();
 
+    test_observer::spawn();
+
+    conf.events_observers.push(EventObserverConfig {
+        endpoint: format!("localhost:{}", test_observer::EVENT_OBSERVER_PORT),
+        events_keys: vec![EventKeyType::AnyEvent],
+    });
+
     let first_bal = 6_000_000_000 * (core::MICROSTACKS_PER_STACKS as u64);
     let second_bal = 2_000_000_000 * (core::MICROSTACKS_PER_STACKS as u64);
     let third_bal = 2_000_000_000 * (core::MICROSTACKS_PER_STACKS as u64);
@@ -917,6 +927,44 @@ fn pox_integration_test() {
         eprintln!("Sort height: {}", sort_height);
     }
 
+    let blocks_observed = test_observer::get_blocks();
+    assert!(
+        blocks_observed.len() >= 2,
+        "Blocks observed {} should be >= 2",
+        blocks_observed.len()
+    );
+
+    // look up the return value of our stacking operation...
+    let mut tested = false;
+    for block in blocks_observed.iter() {
+        if tested {
+            break;
+        }
+        let transactions = block.get("transactions").unwrap().as_array().unwrap();
+        eprintln!("{}", transactions.len());
+        for tx in transactions.iter() {
+            let raw_tx = tx.get("raw_tx").unwrap().as_str().unwrap();
+            if raw_tx == "0x00" {
+                continue;
+            }
+            let tx_bytes = hex_bytes(&raw_tx[2..]).unwrap();
+            let parsed = StacksTransaction::consensus_deserialize(&mut &tx_bytes[..]).unwrap();
+            if let TransactionPayload::ContractCall(_) = parsed.payload {
+            } else {
+                continue;
+            }
+
+            let raw_result = tx.get("raw_result").unwrap().as_str().unwrap();
+            let parsed = <Value as ClarityDeserializable<Value>>::deserialize(&raw_result[2..]);
+            assert_eq!(parsed.to_string(),
+                       format!("(ok (tuple (lock-amount u1000000000000000) (stacker {}) (unlock-burn-height u270)))",
+                               &spender_addr));
+            tested = true;
+        }
+    }
+
+    assert!(tested);
+
     // let's stack with spender 2 and spender 3...
 
     // now let's have sender_2 and sender_3 stack to pox addr 2 in