Merge pull request #234 from jbencin/fix/handle-failed-nft-deposits

fix: Send failed NFT deposits back to user

core-contracts/contracts/helper/simple-nft-l2-no-deposit.clar

@@ -0,0 +1,67 @@
+;; Version of `simple-nft-l2.clar` in which deposit always fails
+
+(define-constant CONTRACT_OWNER tx-sender)
+(define-constant CONTRACT_ADDRESS (as-contract tx-sender))
+
+(define-constant ERR_NOT_AUTHORIZED (err u1001))
+
+(impl-trait 'ST000000000000000000002AMW42H.subnet.nft-trait)
+
+(define-data-var lastId uint u0)
+
+(define-non-fungible-token nft-token uint)
+
+
+;; NFT trait functions
+(define-read-only (get-last-token-id)
+  (ok (var-get lastId))
+)
+
+(define-read-only (get-owner (id uint))
+  (ok (nft-get-owner? nft-token id))
+)
+
+(define-read-only (get-token-uri (id uint))
+  (ok (some "unimplemented"))
+)
+
+(define-public (transfer (id uint) (sender principal) (recipient principal))
+  (begin
+    (asserts! (is-eq tx-sender sender) ERR_NOT_AUTHORIZED)
+    (nft-transfer? nft-token id sender recipient)
+  )
+)
+
+;; mint functions
+(define-public (mint-next (recipient principal))
+  (let
+    ((newId (+ (var-get lastId) u1)))
+    (var-set lastId newId)
+    (nft-mint? nft-token newId recipient)
+  )
+)
+
+(define-public (gift-nft (recipient principal) (id uint))
+  (begin
+    (nft-mint? nft-token id recipient)
+  )
+)
+
+(define-read-only (get-token-owner (id uint))
+  (nft-get-owner? nft-token id)
+)
+
+(impl-trait 'ST000000000000000000002AMW42H.subnet.subnet-asset)
+
+;; Called for deposit from the burnchain to the subnet
+(define-public (deposit-from-burnchain (id uint) (recipient principal))
+  ERR_NOT_AUTHORIZED
+)
+
+;; Called for withdrawal from the subnet to the burnchain
+(define-public (burn-for-withdrawal (id uint) (owner principal))
+  (begin
+    (asserts! (is-eq tx-sender owner) ERR_NOT_AUTHORIZED)
+    (nft-burn? nft-token id owner)
+  )
+)

src/chainstate/stacks/db/blocks.rs

@@ -4710,13 +4710,45 @@ impl StacksChainState {
             .collect()
     }
 
+    fn make_nft_withdrawal_event(
+        subnet_contract_id: QualifiedContractIdentifier,
+        sender: PrincipalData,
+        id: u128,
+        mainnet: bool,
+    ) -> StacksTransactionEvent {
+        StacksTransactionEvent::SmartContractEvent(SmartContractEventData {
+            key: (boot_code_id("subnet", mainnet), "print".into()),
+            value: TupleData::from_data(vec![
+                (
+                    "event".into(),
+                    Value::string_ascii_from_bytes("withdraw".into())
+                        .expect("Supplied string was not ASCII"),
+                ),
+                (
+                    "type".into(),
+                    Value::string_ascii_from_bytes("nft".into())
+                        .expect("Supplied string was not ASCII"),
+                ),
+                ("sender".into(), Value::Principal(sender)),
+                ("id".into(), Value::UInt(id)),
+                (
+                    "asset-contract".into(),
+                    Value::Principal(PrincipalData::Contract(subnet_contract_id)),
+                ),
+            ])
+            .expect("Failed to create tuple data.")
+            .into(),
+        })
+    }
+
     /// Process any deposit NFT operations that haven't been processed in this
     /// subnet fork yet.
     pub fn process_deposit_nft_ops(
         clarity_tx: &mut ClarityTx,
         operations: Vec<DepositNftOp>,
     ) -> Vec<StacksTransactionReceipt> {
         let mainnet = clarity_tx.config.mainnet;
+        let boot_addr = PrincipalData::from(boot_code_addr(mainnet));
         let cost_so_far = clarity_tx.cost_so_far();
         // return valid receipts
         operations
@@ -4732,11 +4764,11 @@ impl StacksChainState {
                 } = deposit_nft_op.clone();
                 let result = clarity_tx.connection().as_transaction(|tx| {
                     tx.run_contract_call(
-                        &boot_code_addr(mainnet).into(),
+                        &boot_addr,
                         None,
                         &subnet_contract_id,
                         DEPOSIT_FUNCTION_NAME,
-                        &[Value::UInt(id), Value::Principal(sender)],
+                        &[Value::UInt(id), Value::Principal(sender.clone())],
                         |_, _| false,
                     )
                 });
@@ -4746,22 +4778,47 @@ impl StacksChainState {
                     .expect("BUG: cost declined between executions");
 
                 match result {
-                    Ok((value, _, events)) => Some(StacksTransactionReceipt {
-                        transaction: TransactionOrigin::Burn(deposit_nft_op.into()),
-                        events,
-                        result: value,
-                        post_condition_aborted: false,
-                        stx_burned: 0,
-                        contract_analysis: None,
-                        execution_cost,
-                        microblock_header: None,
-                        tx_index: 0,
-                    }),
+                    Ok((value, _, mut events)) => {
+                        // Examine response to see if transaction failed
+                        let deposit_op_failed = match &value {
+                            Value::Response(r) => r.committed == false,
+                            _ => {
+                                // TODO: Do anything for the case where `value` is not of type `Value::Response()`?
+                                warn!("DepositNft op returned unexpected value"; "value" => %value);
+                                false
+                            }
+                        };
+
+                        // If deposit fails, create a withdrawal event to send NFT back to user
+                        if deposit_op_failed {
+                            info!("DepositNft op failed. Issue withdrawal tx");
+                            events.push(Self::make_nft_withdrawal_event(
+                                subnet_contract_id,
+                                sender,
+                                id,
+                                mainnet,
+                            ));
+                        };
+
+                        Some(StacksTransactionReceipt {
+                            transaction: TransactionOrigin::Burn(deposit_nft_op.into()),
+                            events,
+                            result: value,
+                            post_condition_aborted: false,
+                            stx_burned: 0,
+                            contract_analysis: None,
+                            execution_cost,
+                            microblock_header: None,
+                            tx_index: 0,
+                        })
+                    }
                     Err(e) => {
+                        // Deposit was not processed, log and do nothing else?
                         info!("DepositNft op processing error.";
                               "error" => ?e,
                               "txid" => %txid,
                               "burn_block" => %burn_header_hash);
+
                         None
                     }
                 }

src/chainstate/stacks/miner.rs

@@ -1805,6 +1805,7 @@ impl StacksBlockBuilder {
             microblock_fees,
             matured_miner_rewards_opt,
             microblock_txs_receipts,
+            tx_receipts,
             ..
         } = StacksChainState::setup_block(
             &mut info.chainstate_tx,
@@ -1820,6 +1821,7 @@ impl StacksBlockBuilder {
             info.mainnet,
             Some(self.miner_id),
         )?;
+        self.tx_receipts.extend(tx_receipts.into_iter());
         self.microblock_tx_receipts = microblock_txs_receipts;
         self.miner_payouts =
             matured_miner_rewards_opt.map(|(miner, users, parent, _)| (miner, users, parent));