Repository navigation
Conversation
Such a tx passes AcceptToMemoryPool, which runs CheckSpecialTx before applying inputs to the view, so GetUTXOCoin still finds the collateral. ConnectBlock runs UpdateCoins (validation.cpp:1602) before ProcessSpecialTxsInBlock (:1671), so by then the coin is spent and validation fails. BlockAssembler throws on that failure instead of dropping the tx (blockassembler.cpp:253), so block production stops for as long as the tx sits in the mempool. Checked ahead of GetUTXOCoin so the reject reason names the real cause at both call sites; after it, ConnectBlock reports the misleading bad-protx-collateral instead. Only the external-collateral branch needs this: when collateralOutpoint.hash is null the collateral is an output of this same tx and cannot also be one of its inputs. The regression test builds a fully valid ProRegTx, collateral ownership signature included. Without that the tx is rejected by CheckStringSig and never reaches the collateral rule. Without this commit CheckSpecialTx returns true for it. Fix extracted from PIVX-Project#2804, where it is one commit inside a 4500-line GUI change.
|
utACK 01165fe Built locally on macOS arm64 and verified with:
All tests passed. I also verified the validation flow and the self-collateral-spend case described in the PR. The fix is minimal and correctly rejects the invalid ProRegTx before it can enter the mempool, while preserving valid external and internal collateral handling. No blocking issues found. |
|
ACK Correct, minimal guard in CheckProRegTx (src/evo/specialtx_validation.cpp:154-158): rejects an external-collateral ProRegTx whose collateral outpoint is also its own input. Matches neighbouring DoS(10)/REJECT_INVALID checks; no consensus risk (ConnectBlock already rejected such blocks). Tested:
Nits: line-numbered comment will drift; test calls CheckSpecialTx directly, so the block-halt claim is argued not E2E-proven; guard precedes CheckInputsHash (DoS-10 vs DoS-100); |
A wallet whose only coin is the collateral makes protx_register fund the fee from that coin, which builds the self-spending ProRegTx through the normal RPC path. The test expects bad-protx-collateral-used-as-input, an empty mempool, and a block still being mined. Without the fix the RPC accepts the tx and the test fails.
A ProRegTx whose VIN includes the collateral outpoint it declares passes full validation today and halts block production when ProRegTx is enabled.
Path
AcceptToMemoryPoolcallsCheckSpecialTx(validation.cpp:491) before the tx's inputs are applied to the view, soGetUTXOCoinstill finds the collateral and the tx is accepted to the mempool.ConnectBlockrunsUpdateCoinsbeforeProcessSpecialTxsInBlock. By then, the collateral is spent,CheckProRegTxfails, andBlockAssemblerthrows on theTestBlockValidityfailure rather than dropping the tx. Block production stops for as long as the tx is in the mempool.existsProviderTxConflictdoes not catch it:mapNextTxrecords other transactions' inputs and cannot see the candidate's own.Fix
Rejecting the tx at mempool time with
bad-protx-collateral-used-as-input.Placed ahead of
GetUTXOCoinso the reject reason names the real cause at both call sites. After it, the ConnectBlock path reportsbad-protx-collateralinstead, which sends you looking at the UTXO set.Only the external-collateral branch needs it. When
collateralOutpoint.hashis null, the collateral is an output of this same tx and cannot also be one of its inputs.Test
The regression test builds a fully valid ProRegTx, collateral ownership signature included.
CreateProRegTxnever setsvchSig, so no existing test constructs a valid external-collateral ProRegTx, and a test without it is rejected byCheckStringSigand never reaches the collateral rule.Without this commit,
CheckSpecialTxreturns true for that tx with an empty reject reason. With it, `bad-protx-collateral-used-asVerified locally: 11/11 cases pass across
determinisevo_specialtx_testsandevo_trivialvalidation`.Origin
Extracted from #2804, where it is one commit inside a 4500-line GUI change. #2804 will drop this on rebase.