Merge pull request 'Reject ztxs with duplicate zkproofs' (#327 ) from duplicate_proofs into dev

Reviewed-on: https://git.hush.is/hush/hush3/pulls/327
Reject ztxs with duplicate zkproofs
2025-07-20 00:01:55 -04:00 · 2023-10-13 11:58:00 +00:00 · 2023-10-13 04:31:41 -07:00 · 2023-10-13 11:18:29 +00:00 · 2023-10-12 10:01:01 -04:00 · 2023-10-11 12:07:40 -04:00
3 changed files with 39 additions and 8 deletions
--- a/src/consensus/validation.h
+++ b/src/consensus/validation.h
@ -29,6 +29,8 @@ static const unsigned char REJECT_MALFORMED = 0x01;
 static const unsigned char REJECT_INVALID = 0x10;
 static const unsigned char REJECT_OBSOLETE = 0x11;
 static const unsigned char REJECT_DUPLICATE = 0x12;
+static const unsigned char REJECT_DUPLICATE_OUTPUT_PROOF = 0x13;
+static const unsigned char REJECT_DUPLICATE_SPEND_PROOF  = 0x14;
 static const unsigned char REJECT_NONSTANDARD = 0x40;
 static const unsigned char REJECT_DUST = 0x41;
 static const unsigned char REJECT_INSUFFICIENTFEE = 0x42;
--- a/src/main.cpp
+++ b/src/main.cpp
@ -1751,7 +1751,31 @@ bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState &state, const CTransa
    {
        return error("AcceptToMemoryPool: CheckTransaction failed");
    }
-    
+
+    // Reject duplicate output proofs in a single ztx in mempool
+    // Migrate this to CheckTransaction() to make it a consensus requirement
+    {
+        set<libzcash::GrothProof> vSaplingOutputProof;
+        BOOST_FOREACH(const OutputDescription& output, tx.vShieldedOutput)
+        {
+            if (vSaplingOutputProof.count(output.zkproof))
+                return state.Invalid(error("AcceptToMemoryPool: duplicate output proof"),REJECT_DUPLICATE_OUTPUT_PROOF, "bad-txns-duplicate-output-proof");
+            vSaplingOutputProof.insert(output.zkproof);
+        }
+    }
+
+    // Reject duplicate spend proofs in a single ztx in mempool
+    // Migrate this to CheckTransaction() to make it a consensus requirement
+    {
+        set<libzcash::GrothProof> vSaplingSpendProof;
+        BOOST_FOREACH(const SpendDescription& spend, tx.vShieldedSpend)
+        {
+            if (vSaplingSpendProof.count(spend.zkproof))
+                return state.Invalid(error("AcceptToMemoryPool: duplicate spend proof"),REJECT_DUPLICATE_SPEND_PROOF, "bad-txns-duplicate-spend-proof");
+            vSaplingSpendProof.insert(spend.zkproof);
+        }
+    }
+
    // DoS level set to 10 to be more forgiving.
    // Check transaction contextually against the set of consensus rules which apply in the next block to be mined.
    if (!ContextualCheckTransaction(0,0,0,tx, state, nextBlockHeight, (dosLevel == -1) ? 10 : dosLevel))
--- a/src/miner.cpp
+++ b/src/miner.cpp
@ -1122,6 +1122,7 @@ void static RandomXMiner()
    int randomxInterval = GetArg("-ac_randomx_interval",1024);
    // This lag is 80 mins for 75s blocktime and 64 mins for 60s (default) blocktime for HSCs
    int randomxBlockLag = GetArg("-ac_randomx_lag", 64);
+    randomx_vm *myVM = nullptr;

    try {
        // fprintf(stderr,"RandomXMiner: mining %s with randomx\n",SMART_CHAIN_SYMBOL);
@ -1198,7 +1199,7 @@ void static RandomXMiner()
            // randomx_init_dataset(randomxDataset, randomxCache, 0, datasetItemCount);
            rxdebug("%s: dataset initialized\n");

-            randomx_vm *myVM = randomx_create_vm(flags, nullptr, randomxDataset);
+            myVM = randomx_create_vm(flags, nullptr, randomxDataset);
            if(myVM == NULL) {
                LogPrintf("RandomXMiner: Cannot create RandomX VM, aborting!\n");
                return;
@ -1425,10 +1426,12 @@ void static RandomXMiner()
       miningTimer.stop();
       c.disconnect();

+       randomx_destroy_vm(myVM);
+       LogPrintf("%s: destroyed vm via thread interrupt\n", __func__);
       randomx_release_dataset(randomxDataset);
-       rxdebug("%s: released dataset\n");
+       rxdebug("%s: released dataset via thread interrupt\n");
       randomx_release_cache(randomxCache);
-       rxdebug("%s: released cache\n");
+       rxdebug("%s: released cache via thread interrupt\n");

       LogPrintf("HushRandomXMiner terminated\n");
       throw;
@ -1437,18 +1440,20 @@ void static RandomXMiner()
       c.disconnect();
       fprintf(stderr,"RandomXMiner: runtime error: %s\n", e.what());

+       randomx_destroy_vm(myVM);
+       LogPrintf("%s: destroyed vm because of error\n", __func__);
       randomx_release_dataset(randomxDataset);
-       rxdebug("%s: released dataset\n");
+       rxdebug("%s: released dataset because of error\n");
       randomx_release_cache(randomxCache);
-       rxdebug("%s: released cache\n");
+       rxdebug("%s: released cache because of error\n");

       return;
   }

   randomx_release_dataset(randomxDataset);
-   rxdebug("%s: released dataset\n");
+   rxdebug("%s: released dataset in normal exit\n");
   randomx_release_cache(randomxCache);
-   rxdebug("%s: released cache\n");
+   rxdebug("%s: released cache in normal exit\n");
   miningTimer.stop();
   c.disconnect();
 }
Author	SHA1	Message	Date
duke	fb5563d06a	Merge pull request 'Reject ztxs with duplicate zkproofs' (#327 ) from duplicate_proofs into dev Reviewed-on: https://git.hush.is/hush/hush3/pulls/327	2023-10-13 11:58:00 +00:00
Duke	14d3ae1785	Reject ztxs with duplicate zkproofs This is a greatly simplified and slightly tweaked version of `af2e3713e2` Their version will detect duplicate zkproofs across transactions while this code will only detect duplicate zkproofs in a single ztx. If dupes are found, the tx will be denied entry into the mempool. This provides most of the benefit (increased CPU cost to attackers) with the least code change and no annoyance to full node operators. Detecting duplicate zkproofs across transactions requires a one-time reindex of all of history, which means significant downtime for nodes. Since Hush + HSCs have a much more strict policy on number of shielded outputs and shielded inputs, only detecting duplicate zkproofs in individual ztxs seems sufficient for now. No correctly functioning node or wallet will ever create duplicate zkproofs, so there is no worry of this accidentally affecting normal users. Currently this is not a consensus rule but it could become one in the future.	2023-10-13 04:31:41 -07:00
duke	70fa319fd9	Merge pull request 'Fix randomx mining memleak' (#326 ) from memleak into dev Reviewed-on: https://git.hush.is/hush/hush3/pulls/326	2023-10-13 11:18:29 +00:00
Duke	80bd3f262c	Verbosify randomx debug logging in case that helps debug mismatched height coinbase issue	2023-10-12 10:01:01 -04:00
Duke	fc6745129d	Fix randomx memory leak but create some mining errors This change lifts the declaration of the randomx VM out of an inner loop into the main function body of RandomXMiner(), which allows us to destroy it later on when catching exceptions. We cannot lift the allocation of it's memory (randomx_create_vm) because it depends on things that change in every iteration of the inner loop. Otherwise, the VM will only sometimes be destroyed, which is what I think causes the memleak. But this seems to create one invalid block when mining each block height : STDOUT: TestBlockValidity: failure C checkPOW=1 RandomXMiner: Invalid randomx block mined, try again 05f30f419133b2d862106b89c20059967639e4f2699dd5afc5d2b0832f1ac76a debug.log: 2023-10-11 16:10:41 CreateNewBlock(): total size 1000 blocktime.1697040642 nBits.200e77d1 2023-10-11 16:10:41 Running HushRandomXMiner with 1 transactions in block (260 bytes) 2023-10-11 16:10:41 ERROR: ContextualCheckBlock: block height mismatch in coinbase Mining does seem to continue normally when testing with -testnode=1	2023-10-11 12:07:40 -04:00