From 74987d30b1b52eebd30effbefd2d43282ae31489 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Sat, 30 May 2015 00:37:07 -0700 Subject: [PATCH] Require non-hidden values in sidechain withdraws/reorgs --- src/main.cpp | 3 ++- src/script/interpreter.cpp | 34 ++++++++++++++++++++++++---------- src/script/script_error.cpp | 4 ++++ src/script/script_error.h | 2 ++ 4 files changed, 32 insertions(+), 11 deletions(-) diff --git a/src/main.cpp b/src/main.cpp index 6dea8150d3..a21615adb3 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -1978,7 +1978,8 @@ bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pin // Because miners can take it anyway, we just devote the whole fraud bounty to miner fee const CScript &withdrawOutputScript = tx.vout[j].scriptPubKey; assert(withdrawOutputScript.IsWithdrawOutput()); - proofTx.vout[0].nValue = tx.vout[j].nValue - withdrawOutputScript.GetFraudBounty(); + assert(tx.vout[j].nValue.IsAmount()); + proofTx.vout[0].nValue = tx.vout[j].nValue.GetAmount() - withdrawOutputScript.GetFraudBounty(); pvProofTxn->push_back(proofTx); } diff --git a/src/script/interpreter.cpp b/src/script/interpreter.cpp index ad0f41009b..f98bb3fd81 100644 --- a/src/script/interpreter.cpp +++ b/src/script/interpreter.cpp @@ -1093,13 +1093,19 @@ bool EvalScript(vector >& stack, const CScript& script, un assert(checker.GetValueIn() != -1); // Not using a NoWithdrawSignatureChecker if (stack.size() == 2) { // increasing value of locked coins - CAmount minValue = checker.GetValueIn(); + if (!checker.GetValueIn().IsAmount()) + return set_error(serror, SCRIPT_ERR_WITHDRAW_VALUES_HIDDEN); + CAmount minValue = checker.GetValueIn().GetAmount(); CTxOut newOutput = checker.GetOutputOffsetFromCurrent(0); if (newOutput.IsNull()) { newOutput = checker.GetOutputOffsetFromCurrent(-1); - minValue += checker.GetValueInPrevIn(); + if (!checker.GetValueInPrevIn().IsAmount()) + return set_error(serror, SCRIPT_ERR_WITHDRAW_VALUES_HIDDEN); + minValue += checker.GetValueInPrevIn().GetAmount(); } - if (newOutput.scriptPubKey != script || newOutput.nValue < minValue) + if (!newOutput.nValue.IsAmount()) + return set_error(serror, SCRIPT_ERR_WITHDRAW_VALUES_HIDDEN); + if (newOutput.scriptPubKey != script || newOutput.nValue.GetAmount() < minValue) return set_error(serror, SCRIPT_ERR_WITHDRAW_VERIFY_OUTPUT); } else { // stack.size() == 10...ie regular withdraw int stackReadPos = -3; @@ -1206,7 +1212,7 @@ bool EvalScript(vector >& stack, const CScript& script, un unsigned char *pub_start = &(*(sdpc - 33)); CHMAC_SHA256(pub_start, 33).Write(&vcontract[0], 40).Finalize(tweak); // If someone creates a tweak that makes this fail, they broke SHA256 - assert(secp256k1_ec_pubkey_tweak_add(pub_start, 33, tweak) != 0); + assert(secp256k1_ec_pubkey_tweak_add(secp256k1_context, pub_start, 33, tweak) != 0); } } } @@ -1231,13 +1237,19 @@ bool EvalScript(vector >& stack, const CScript& script, un if (vcontract[0] != 'P' || vcontract[1] != '2' || vcontract[2] != 'S' || vcontract[3] != 'H') return set_error(serror, SCRIPT_ERR_WITHDRAW_VERIFY_FORMAT); - CAmount withdrawVal = locktx.vout[nlocktxOut].nValue; + if (!locktx.vout[nlocktxOut].nValue.IsAmount()) + return set_error(serror, SCRIPT_ERR_WITHDRAW_VALUES_HIDDEN); + CAmount withdrawVal = locktx.vout[nlocktxOut].nValue.GetAmount(); const CTxOut newLockOutput = checker.GetOutputOffsetFromCurrent(1); - if (newLockOutput.scriptPubKey != script || newLockOutput.nValue < checker.GetValueIn() - withdrawVal) + if (!newLockOutput.nValue.IsAmount() || !checker.GetValueIn().IsAmount()) + return set_error(serror, SCRIPT_ERR_WITHDRAW_VALUES_HIDDEN); + if (newLockOutput.scriptPubKey != script || newLockOutput.nValue.GetAmount() < checker.GetValueIn().GetAmount() - withdrawVal) return set_error(serror, SCRIPT_ERR_WITHDRAW_VERIFY_OUTPUT); const CTxOut withdrawOutput = checker.GetOutputOffsetFromCurrent(0); - if (withdrawOutput.nValue < withdrawVal) + if (!withdrawOutput.nValue.IsAmount()) + return set_error(serror, SCRIPT_ERR_WITHDRAW_VALUES_HIDDEN); + if (withdrawOutput.nValue.GetAmount() < withdrawVal) return set_error(serror, SCRIPT_ERR_WITHDRAW_VERIFY_OUTPUT); uint256 locktxHash = locktx.GetHash(); @@ -1247,7 +1259,7 @@ bool EvalScript(vector >& stack, const CScript& script, un #ifndef FEDERATED_PEG_SIDECHAIN_ONLY << 42 // (TODO: Measure of work from genesis to proof tip) #endif - << CScriptNum(withdrawOutput.nValue - withdrawVal) // Fraud bounty + << CScriptNum(withdrawOutput.nValue.GetAmount() - withdrawVal) // Fraud bounty << vsecondScriptPubKeyHash << vgenesisHash << OP_REORGPROOFVERIFY << OP_ELSE; // << lockTime << OP_CHECKSEQUENCEVERIFY << OP_DROP << OP_HASH160 // << std::vector(vcontract.begin() + 4, vcontract.begin() + 24) << OP_EQUAL << OP_ENDIF; @@ -1291,7 +1303,7 @@ bool EvalScript(vector >& stack, const CScript& script, un // 3. relative locktime // 4. <1> indicating we are checking a withdraw proof withdrawStack.push_back(CScriptNum(checker.GetTransactionFee()).getvch()); - withdrawStack.push_back(CScriptNum(withdrawOutput.nValue - withdrawVal).getvch()); + withdrawStack.push_back(CScriptNum(withdrawOutput.nValue.GetAmount() - withdrawVal).getvch()); withdrawStack.push_back(vlockTime); withdrawStack.push_back(std::vector(1, 1)); #ifndef FEDERATED_PEG_SIDECHAIN_ONLY @@ -1453,7 +1465,9 @@ bool EvalScript(vector >& stack, const CScript& script, un if (newLockOutput.scriptPubKey != (CScript() << vgenesisHash << vsecondScriptPubKeyHash << OP_WITHDRAWPROOFVERIFY)) return set_error(serror, SCRIPT_ERR_REORG_VERIFY_FRAUD_OUTPUT); CAmount fraudBounty = CScriptNum(vfraudBountyValue, fRequireMinimal, 8).getint64(); - if (newLockOutput.nValue < checker.GetValueIn() - fraudBounty) + if (!newLockOutput.nValue.IsAmount() || !checker.GetValueIn().IsAmount()) + return set_error(serror, SCRIPT_ERR_REORG_VALUES_HIDDEN); + if (newLockOutput.nValue.GetAmount() < checker.GetValueIn().GetAmount() - fraudBounty) return set_error(serror, SCRIPT_ERR_REORG_VERIFY_FRAUD_OUTPUT); //TODO: if (!checker.IsInBlockTreeAboveMe(merkleBlock.header.GetHash())) diff --git a/src/script/script_error.cpp b/src/script/script_error.cpp index 3505500279..cc9e0f482c 100644 --- a/src/script/script_error.cpp +++ b/src/script/script_error.cpp @@ -81,6 +81,8 @@ const char* ScriptErrorString(const ScriptError serror) return "Withdraw proof validation failed - second script validation failed"; case SCRIPT_ERR_WITHDRAW_VERIFY_BLOCKCONFIRMED: return "Withdraw proof validation failed - lock block was not sufficiently confirmed on sending chain"; + case SCRIPT_ERR_WITHDRAW_VALUES_HIDDEN: + return "Withdraw proof validation failed - values were hidden"; case SCRIPT_ERR_REORG_VERIFY_FORMAT: return "Reorg/Fraud proof validation failed - invalid proof format"; case SCRIPT_ERR_REORG_VERIFY_FRAUD_BLOCK: @@ -91,6 +93,8 @@ const char* ScriptErrorString(const ScriptError serror) return "Fraud proof validation failed - bad or unmatched original withdraw tx"; case SCRIPT_ERR_REORG_VERIFY_FRAUD_OUTPUT: return "Fraud proof validation failed - output does not match expected"; + case SCRIPT_ERR_REORG_VALUES_HIDDEN: + return "Fraud proof validation failed - values were hidden"; case SCRIPT_ERR_UNKNOWN_ERROR: case SCRIPT_ERR_ERROR_COUNT: default: break; diff --git a/src/script/script_error.h b/src/script/script_error.h index 58fda930ab..75fec1b9ab 100644 --- a/src/script/script_error.h +++ b/src/script/script_error.h @@ -59,11 +59,13 @@ typedef enum ScriptError_t SCRIPT_ERR_WITHDRAW_VERIFY_LOCKTIME, SCRIPT_ERR_WITHDRAW_VERIFY_SECONDSCRIPT, SCRIPT_ERR_WITHDRAW_VERIFY_BLOCKCONFIRMED, + SCRIPT_ERR_WITHDRAW_VALUES_HIDDEN, SCRIPT_ERR_REORG_VERIFY_FORMAT, SCRIPT_ERR_REORG_VERIFY_FRAUD_BLOCK, SCRIPT_ERR_REORG_VERIFY_FRAUD_ORIG_BLOCK, SCRIPT_ERR_REORG_VERIFY_FRAUD_ORIG_TX, SCRIPT_ERR_REORG_VERIFY_FRAUD_OUTPUT, + SCRIPT_ERR_REORG_VALUES_HIDDEN, SCRIPT_ERR_ERROR_COUNT } ScriptError;