From f12c29da99659bf61d2ec35f5386c7ed43c0bf93 Mon Sep 17 00:00:00 2001 From: ArokyaMatthew Date: Sun, 5 Apr 2026 07:36:18 +0530 Subject: [PATCH 1/2] [UTXO-BUG] LOW: Duplicate input dedup, missing validations, and test coverage gaps LOW-2: apply_transaction() allows the same box_id twice in inputs, inflating input_total 2x. The spend-phase rowcount catches it today but only accidentally. Added explicit dedup check. LOW-3: spending_proof field is never verified in the UTXO layer. Documented with a test so future maintainers know sig verification is the endpoint layer's responsibility. LOW-4: Added tests for previously uncovered scenarios: - test_duplicate_input_rejected (defense-in-depth dedup) - test_self_transfer (from == to edge case) - test_spending_proof_not_verified_in_utxo_layer (documents behavior) All 37 tests pass. Bounty: #2819 (Low, 25 RTC x3) --- node/test_utxo_db.py | 56 ++++++++++++++++++++++++++++++++++++++++++++ node/utxo_db.py | 9 +++++++ 2 files changed, 65 insertions(+) diff --git a/node/test_utxo_db.py b/node/test_utxo_db.py index 78352f80..c14e22cf 100644 --- a/node/test_utxo_db.py +++ b/node/test_utxo_db.py @@ -343,6 +343,62 @@ def test_mining_reward_empty_inputs_allowed(self): self.assertTrue(ok) self.assertEqual(self.db.get_balance('alice'), 100 * UNIT) + # -- bounty #2819 LOW: validation gaps & edge cases ---------------------- + + def test_duplicate_input_rejected(self): + """Same box_id listed twice in inputs must be rejected. + Without explicit dedup, input_total is inflated 2x (LOW-2).""" + self._apply_coinbase('alice', 100 * UNIT) + boxes = self.db.get_unspent_for_address('alice') + box_id = boxes[0]['box_id'] + + ok = self.db.apply_transaction({ + 'tx_type': 'transfer', + 'inputs': [ + {'box_id': box_id, 'spending_proof': 'sig'}, + {'box_id': box_id, 'spending_proof': 'sig'}, # duplicate + ], + 'outputs': [{'address': 'attacker', 'value_nrtc': 200 * UNIT}], + 'fee_nrtc': 0, + }, block_height=10) + self.assertFalse(ok) + self.assertEqual(self.db.get_balance('alice'), 100 * UNIT) + self.assertEqual(self.db.get_balance('attacker'), 0) + + def test_self_transfer(self): + """Self-transfer (from == to) must work correctly.""" + self._apply_coinbase('alice', 100 * UNIT) + boxes = self.db.get_unspent_for_address('alice') + + ok = self.db.apply_transaction({ + 'tx_type': 'transfer', + 'inputs': [{'box_id': boxes[0]['box_id'], + 'spending_proof': 'sig'}], + 'outputs': [{'address': 'alice', 'value_nrtc': 100 * UNIT}], + 'fee_nrtc': 0, + }, block_height=10) + self.assertTrue(ok) + self.assertEqual(self.db.get_balance('alice'), 100 * UNIT) + + def test_spending_proof_not_verified_in_utxo_layer(self): + """The UTXO layer does NOT verify spending proofs — by design. + Signature verification is the endpoint layer's responsibility. + This test documents the behavior so future changes don't + accidentally rely on it (LOW-3).""" + self._apply_coinbase('alice', 100 * UNIT) + boxes = self.db.get_unspent_for_address('alice') + + # Bogus spending_proof is accepted at the UTXO layer + ok = self.db.apply_transaction({ + 'tx_type': 'transfer', + 'inputs': [{'box_id': boxes[0]['box_id'], + 'spending_proof': 'TOTALLY_BOGUS'}], + 'outputs': [{'address': 'bob', 'value_nrtc': 100 * UNIT}], + 'fee_nrtc': 0, + }, block_height=10) + self.assertTrue(ok, "UTXO layer should accept any spending_proof " + "(verification is endpoint's job)") + def test_mempool_empty_inputs_rejected_for_transfer(self): """Mempool must also reject non-minting txs with empty inputs.""" tx = { diff --git a/node/utxo_db.py b/node/utxo_db.py index db77aaf1..dbb0fe39 100644 --- a/node/utxo_db.py +++ b/node/utxo_db.py @@ -316,6 +316,15 @@ def apply_transaction(self, tx: dict, block_height: int, try: conn.execute("BEGIN IMMEDIATE") + # -- reject duplicate input box_ids -------------------------------- + # Without this, the same box_id counted twice inflates + # input_total. The spend-phase rowcount check catches it + # today, but only accidentally. Defense in depth. + input_box_ids = [i['box_id'] for i in inputs] + if len(input_box_ids) != len(set(input_box_ids)): + conn.execute("ROLLBACK") + return False + # -- validate inputs exist and are unspent ----------------------- input_total = 0 for inp in inputs: From 6be9371399fa94f0cd0c0be137968578fd609414 Mon Sep 17 00:00:00 2001 From: ArokyaMatthew Date: Sun, 5 Apr 2026 16:30:34 +0530 Subject: [PATCH 2/2] =?UTF-8?q?Address=20reviewer=20feedback:=20docstring?= =?UTF-8?q?=20warning=20+=20test=20rename=20-=20Add=20warning=20in=20apply?= =?UTF-8?q?=5Ftransaction()=20docstring:=20spending=5Fproof=20is=20NOT=20?= =?UTF-8?q?=20=20verified=20at=20this=20layer,=20callers=20MUST=20check=20?= =?UTF-8?q?signatures=20first=20=20=20(per=20@zhuzhushiwojia=20suggestion)?= =?UTF-8?q?=20-=20Rename=20test=20to=20test=5Fspending=5Fproof=5Faccepted?= =?UTF-8?q?=5Fwithout=5Fverification=20=20=20(per=20@geldbert=20suggestion?= =?UTF-8?q?=20=E2=80=94=20clearer=20intent)=20-=20Document=20dedup=20is=20?= =?UTF-8?q?keyed=20on=20box=5Fid=20alone=20(per=20@geldbert=20observation)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- node/test_utxo_db.py | 4 ++-- node/utxo_db.py | 9 +++++++++ 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/node/test_utxo_db.py b/node/test_utxo_db.py index c14e22cf..355de931 100644 --- a/node/test_utxo_db.py +++ b/node/test_utxo_db.py @@ -380,8 +380,8 @@ def test_self_transfer(self): self.assertTrue(ok) self.assertEqual(self.db.get_balance('alice'), 100 * UNIT) - def test_spending_proof_not_verified_in_utxo_layer(self): - """The UTXO layer does NOT verify spending proofs — by design. + def test_spending_proof_accepted_without_verification(self): + """The UTXO layer accepts any spending_proof without verification. Signature verification is the endpoint layer's responsibility. This test documents the behavior so future changes don't accidentally rely on it (LOW-3).""" diff --git a/node/utxo_db.py b/node/utxo_db.py index dbb0fe39..65470e17 100644 --- a/node/utxo_db.py +++ b/node/utxo_db.py @@ -292,6 +292,12 @@ def apply_transaction(self, tx: dict, block_height: int, """ Atomically apply a transaction: spend inputs, create outputs. + .. warning:: + This method does **not** verify ``spending_proof``. Callers + MUST authenticate the spender (e.g. Ed25519 signature check) + before calling this method. See ``utxo_endpoints.py`` for + the endpoint-level verification. + ``tx`` keys: tx_type: str inputs: list of {box_id: str, spending_proof: str} @@ -317,6 +323,9 @@ def apply_transaction(self, tx: dict, block_height: int, conn.execute("BEGIN IMMEDIATE") # -- reject duplicate input box_ids -------------------------------- + # Keyed on box_id alone (the PK of the UTXO being consumed). + # Different spending_proof values for the same box_id are still + # a duplicate — the proof content is irrelevant to dedup. # Without this, the same box_id counted twice inflates # input_total. The spend-phase rowcount check catches it # today, but only accidentally. Defense in depth.