[chore] Reject duplicate deposit requests when first one already acce…

…pted | confirmed | failed (#1179)

* wip, started changing handler

* test works now

* enrich error message

* fix typo

* review notes

* add Pending and Reprocessing statuses to test

* make comment more beautiul

* keep stacks block height and hash for pending dups & get_deposit_entry error handling

* use test_case macro

* move import to file top

* small refactoring

.generated-sources/emily/openapi/generated-specs/private-emily-openapi-spec.json

@@ -510,6 +510,16 @@
               }
             }
           },
+          "409": {
+            "description": "Duplicate request",
+            "content": {
+              "application/json": {
+                "schema": {
+                  "$ref": "#/components/schemas/ErrorResponse"
+                }
+              }
+            }
+          },
           "500": {
             "description": "Internal server error",
             "content": {

.generated-sources/emily/openapi/generated-specs/public-emily-openapi-spec.json

@@ -429,6 +429,16 @@
               }
             }
           },
+          "409": {
+            "description": "Duplicate request",
+            "content": {
+              "application/json": {
+                "schema": {
+                  "$ref": "#/components/schemas/ErrorResponse"
+                }
+              }
+            }
+          },
           "500": {
             "description": "Internal server error",
             "content": {

.generated-sources/emily/openapi/generated-specs/testing-emily-openapi-spec.json

@@ -510,6 +510,16 @@
               }
             }
           },
+          "409": {
+            "description": "Duplicate request",
+            "content": {
+              "application/json": {
+                "schema": {
+                  "$ref": "#/components/schemas/ErrorResponse"
+                }
+              }
+            }
+          },
           "500": {
             "description": "Internal server error",
             "content": {

emily/handler/src/api/handlers/deposit.rs

@@ -8,9 +8,6 @@ use stacks_common::codec::StacksMessageCodec as _;
 use tracing::{debug, instrument};
 use warp::reply::{json, with_status, Reply};
 
-use bitcoin::ScriptBuf;
-use warp::http::StatusCode;
-
 use crate::api::models::deposit::{Deposit, DepositInfo};
 use crate::api::models::{
     deposit::requests::{
@@ -26,6 +23,8 @@ use crate::database::entries::deposit::{
     DepositEntry, DepositEntryKey, DepositEvent, DepositParametersEntry,
     ValidatedUpdateDepositsRequest,
 };
+use bitcoin::ScriptBuf;
+use warp::http::StatusCode;
 
 /// Get deposit handler.
 #[utoipa::path(
@@ -199,6 +198,7 @@ pub async fn get_deposits(
         (status = 400, description = "Invalid request body", body = ErrorResponse),
         (status = 404, description = "Address not found", body = ErrorResponse),
         (status = 405, description = "Method not allowed", body = ErrorResponse),
+        (status = 409, description = "Duplicate request", body = ErrorResponse),
         (status = 500, description = "Internal server error", body = ErrorResponse)
     )
 )]
@@ -218,8 +218,33 @@ pub async fn create_deposit(
         api_state.error_if_reorganizing()?;
 
         let chaintip = api_state.chaintip();
-        let stacks_block_hash: String = chaintip.key.hash;
-        let stacks_block_height: u64 = chaintip.key.height;
+        let mut stacks_block_hash: String = chaintip.key.hash;
+        let mut stacks_block_height: u64 = chaintip.key.height;
+
+        // Check if deposit with such txid and outindex already exists.
+        let entry = accessors::get_deposit_entry(
+            &context,
+            &DepositEntryKey {
+                bitcoin_txid: body.bitcoin_txid.clone(),
+                bitcoin_tx_output_index: body.bitcoin_tx_output_index,
+            },
+        )
+        .await;
+        // Reject if we already have a deposit with the same txid and output index and it is NOT pending or reprocessing.
+        match entry {
+            Ok(deposit) => {
+                if deposit.status != Status::Pending && deposit.status != Status::Reprocessing {
+                    return Err(Error::Conflict);
+                } else {
+                    // If the deposit is pending or reprocessing, we should keep height and hash same as in the old deposit
+                    stacks_block_hash = deposit.last_update_block_hash;
+                    stacks_block_height = deposit.last_update_height;
+                }
+            }
+            Err(Error::NotFound) => {}
+            Err(e) => return Err(e),
+        }
+
         let status = Status::Pending;
 
         // Get parameters from scripts.

emily/handler/tests/integration/deposit.rs

@@ -1,4 +1,5 @@
 use std::cmp::Ordering;
+use test_case::test_case;
 
 use sbtc::testing;
 use sbtc::testing::deposits::TxSetup;
@@ -548,3 +549,106 @@ async fn update_deposits_updates_chainstate() {
         );
     }
 }
+
+#[tokio::test]
+#[test_case(Status::Pending, false; "Should not reject pending duplicate")]
+#[test_case(Status::Reprocessing, false; "Should not reject reprocessing duplicate")]
+#[test_case(Status::Confirmed, true; "Should reject confirmed duplicate")]
+#[test_case(Status::Failed, true; "Should reject failed duplicate")]
+#[test_case(Status::Accepted, true; "Should reject accepted duplicate")]
+#[cfg_attr(not(feature = "integration-tests"), ignore)]
+async fn overwrite_deposit(status: Status, should_reject: bool) {
+    let configuration = clean_setup().await;
+    // Arrange.
+    // --------
+    let bitcoin_txid: &str = "bitcoin_txid_overwrite_deposit";
+    let bitcoin_tx_output_index = 0;
+
+    // Setup test deposit transaction.
+    let DepositTxnData {
+        reclaim_script, deposit_script, ..
+    } = DepositTxnData::new(DEPOSIT_LOCK_TIME, DEPOSIT_MAX_FEE, DEPOSIT_AMOUNT_SATS);
+
+    let create_deposit_body = CreateDepositRequestBody {
+        bitcoin_tx_output_index,
+        bitcoin_txid: bitcoin_txid.into(),
+        deposit_script: deposit_script.clone(),
+        reclaim_script: reclaim_script.clone(),
+    };
+
+    apis::deposit_api::create_deposit(&configuration, create_deposit_body.clone())
+        .await
+        .expect("Received an error after making a valid create deposit request api call.");
+
+    let response = apis::deposit_api::get_deposit(
+        &configuration,
+        bitcoin_txid,
+        &bitcoin_tx_output_index.to_string(),
+    )
+    .await
+    .expect("Received an error after making a valid get deposit api call.");
+    assert_eq!(response.bitcoin_txid, bitcoin_txid);
+    assert_eq!(response.status, Status::Pending);
+
+    let mut fulfillment: Option<Option<Box<Fulfillment>>> = None;
+
+    if status == Status::Confirmed {
+        fulfillment = Some(Some(Box::new(Fulfillment {
+            bitcoin_block_hash: "bitcoin_block_hash".to_string(),
+            bitcoin_block_height: 23,
+            bitcoin_tx_index: 45,
+            bitcoin_txid: "test_fulfillment_bitcoin_txid".to_string(),
+            btc_fee: 2314,
+            stacks_txid: "test_fulfillment_stacks_txid".to_string(),
+        })));
+    }
+
+    apis::deposit_api::update_deposits(
+        &configuration,
+        UpdateDepositsRequestBody {
+            deposits: vec![DepositUpdate {
+                bitcoin_tx_output_index: bitcoin_tx_output_index,
+                bitcoin_txid: bitcoin_txid.into(),
+                fulfillment,
+                last_update_block_hash: "update_block_hash".into(),
+                last_update_height: 34,
+                status,
+                status_message: "foo".into(),
+            }],
+        },
+    )
+    .await
+    .expect("Received an error after making a valid update deposit request api call.");
+
+    let response = apis::deposit_api::get_deposit(
+        &configuration,
+        bitcoin_txid,
+        &bitcoin_tx_output_index.to_string(),
+    )
+    .await
+    .expect("Received an error after making a valid get deposit api call.");
+    assert_eq!(response.bitcoin_txid, bitcoin_txid);
+    assert_eq!(response.status, status);
+
+    assert_eq!(
+        apis::deposit_api::create_deposit(&configuration, create_deposit_body)
+            .await
+            .is_err(),
+        should_reject
+    );
+
+    let response = apis::deposit_api::get_deposit(
+        &configuration,
+        bitcoin_txid,
+        &bitcoin_tx_output_index.to_string(),
+    )
+    .await
+    .expect("Received an error after making a valid get deposit api call.");
+    assert_eq!(response.bitcoin_txid, bitcoin_txid);
+    let expected_status = if status == Status::Reprocessing {
+        Status::Pending
+    } else {
+        status
+    };
+    assert_eq!(response.status, expected_status);
+}