From ab5b376b72e47a710eff4d79402679b93b383a56 Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Sun, 19 Sep 2021 13:47:20 +0000 Subject: [PATCH 01/11] SignatureHash: fix rangeproof hash for SIGHASH_RANGEPROOF The logic for computing the uncached version of the sighash changed during the 0.21 rebase, such that it no longer matched the cached version. As the changed hash is used during signing (not verification!), this was not a forking change (and our existing functional test would have caught such a forking change since it uses Python to independently compute the hash). But it still broke signing. Test in next commit. --- src/script/interpreter.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/script/interpreter.cpp b/src/script/interpreter.cpp index 9b96564502..211cae7e46 100644 --- a/src/script/interpreter.cpp +++ b/src/script/interpreter.cpp @@ -2755,7 +2755,7 @@ uint256 SignatureHash(const CScript& scriptCode, const T& txTo, unsigned int nIn hashOutputs = cacheready ? cache->hashOutputs : SHA256Uint256(GetOutputsSHA256(txTo)); if (fRangeproof) { - hashRangeproofs = cacheready ? cache->hashRangeproofs : SHA256Uint256(GetRangeproofsHash(txTo)); + hashRangeproofs = cacheready ? cache->hashRangeproofs : GetRangeproofsHash(txTo); } } else if ((nHashType & 0x1f) == SIGHASH_SINGLE && nIn < txTo.vout.size()) { CHashWriter ss(SER_GETHASH, 0); From ac54a2f765a1697752468258ed6fb18ac4794cfa Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Sun, 19 Sep 2021 14:08:24 +0000 Subject: [PATCH 02/11] test: check that signing with the RANGEPROOF flag works --- test/functional/feature_sighash_rangeproof.py | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/test/functional/feature_sighash_rangeproof.py b/test/functional/feature_sighash_rangeproof.py index 5600f6872d..f0789168e6 100755 --- a/test/functional/feature_sighash_rangeproof.py +++ b/test/functional/feature_sighash_rangeproof.py @@ -103,7 +103,8 @@ class SighashRangeproofTest(BitcoinTestFramework): privkey.set(b[0:32], len(b) == 33) pubkey = privkey.get_pubkey() - # Now we need to replace the signature with an equivalent one with the new sighash set. + # Now we need to replace the signature with an equivalent one with the new sighash set, + # which we do using the Python logic to detect any forking changes in the sighash format. hashtype = SIGHASH_ALL | SIGHASH_RANGEPROOF if address_type == "legacy": if sighash_rangeproof_aware: @@ -130,7 +131,20 @@ class SighashRangeproofTest(BitcoinTestFramework): else: assert False - signed_tx.rehash() + # Make sure that the tx we manually signed is valid + test_accept = self.nodes[0].testmempoolaccept([signed_hex])[0] + assert test_accept["allowed"], "not accepted: {}".format(test_accept["reject-reason"]) + + if sighash_rangeproof_aware: + signed_hex = self.nodes[1].signrawtransactionwithwallet(blinded_hex, [], "ALL|RANGEPROOF")["hex"] + signed_tx = FromHex(CTransaction(), signed_hex) + + # Make sure that the tx that the node signed is valid + test_accept = self.nodes[0].testmempoolaccept([signed_hex])[0] + assert test_accept["allowed"], "not accepted: {}".format(test_accept["reject-reason"]) + else: + signed_tx.rehash() + return signed_tx def assert_tx_standard(self, tx, assert_standard=True): From 7a3cbda84899191f3d6397b54aadd1686becfc5c Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Sun, 19 Sep 2021 13:49:40 +0000 Subject: [PATCH 03/11] sign.cpp: turn on SIGHASH_SCRIPT_RANGEPROOF when checking already-existing signatures Currently when a user uses `signrawtransaction` on a transaction that is already partially signed with SIGHASH_RANGEPROOF signatures, the existing signatures will be deleted because they are not recognized as valid. This makes it impossible to collabratively sign transactions using this RPC. Fix this. --- src/script/sign.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/script/sign.cpp b/src/script/sign.cpp index 940bd6a65e..83af10e1a3 100644 --- a/src/script/sign.cpp +++ b/src/script/sign.cpp @@ -310,7 +310,7 @@ SignatureData DataFromTransaction(const CMutableTransaction& tx, unsigned int nI // Get signatures MutableTransactionSignatureChecker tx_checker(&tx, nIn, txout.nValue); SignatureExtractorChecker extractor_checker(data, tx_checker); - if (VerifyScript(data.scriptSig, txout.scriptPubKey, &data.scriptWitness, STANDARD_SCRIPT_VERIFY_FLAGS, extractor_checker)) { + if (VerifyScript(data.scriptSig, txout.scriptPubKey, &data.scriptWitness, STANDARD_SCRIPT_VERIFY_FLAGS | SCRIPT_SIGHASH_RANGEPROOF, extractor_checker)) { data.complete = true; return data; } From 7d39e430c720ed1decc39235e20b7d5f5d1898a9 Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Sun, 19 Sep 2021 14:13:33 +0000 Subject: [PATCH 04/11] test: check that `signrawtransaction` does not erase RANGEPROOF signatures --- test/functional/feature_sighash_rangeproof.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/test/functional/feature_sighash_rangeproof.py b/test/functional/feature_sighash_rangeproof.py index f0789168e6..661312cae2 100755 --- a/test/functional/feature_sighash_rangeproof.py +++ b/test/functional/feature_sighash_rangeproof.py @@ -142,6 +142,11 @@ class SighashRangeproofTest(BitcoinTestFramework): # Make sure that the tx that the node signed is valid test_accept = self.nodes[0].testmempoolaccept([signed_hex])[0] assert test_accept["allowed"], "not accepted: {}".format(test_accept["reject-reason"]) + + # Try re-signing with node 0, which should have no effect since the transaction was already complete + signed_hex = self.nodes[0].signrawtransactionwithwallet(signed_hex)["hex"] + test_accept = self.nodes[0].testmempoolaccept([signed_hex])[0] + assert test_accept["allowed"], "not accepted: {}".format(test_accept["reject-reason"]) else: signed_tx.rehash() From 9c3c61aa0ac211ae4109d533963f651368eaecb4 Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Sun, 19 Sep 2021 13:51:00 +0000 Subject: [PATCH 05/11] psbt: make sure rangeproofs are present in the confidential version of the unsigned tx These rangeproofs are covered by signature in both Taproot and SIGHASH_RANGEPROOF signatures, so they need to be present. --- src/psbt.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/psbt.cpp b/src/psbt.cpp index b788263201..b3272988e9 100644 --- a/src/psbt.cpp +++ b/src/psbt.cpp @@ -133,19 +133,22 @@ CMutableTransaction PartiallySignedTransaction::GetUnsignedTx(bool force_unblind } for (const PSBTOutput& output : outputs) { CTxOut txout; + CTxOutWitness txoutwit; txout.scriptPubKey = *output.script; if (output.IsFullyBlinded() && !force_unblinded) { txout.nValue = output.m_value_commitment; txout.nAsset = output.m_asset_commitment; txout.nNonce.vchCommitment.insert(txout.nNonce.vchCommitment.end(), output.m_ecdh_pubkey.begin(), output.m_ecdh_pubkey.end()); + txoutwit.vchRangeproof = output.m_value_rangeproof; + txoutwit.vchSurjectionproof = output.m_asset_surjection_proof; } else { txout.nValue.SetToAmount(*output.amount); txout.nAsset.SetToAsset(CAsset(output.m_asset)); } mtx.vout.push_back(txout); + mtx.witness.vtxoutwit.push_back(txoutwit); } mtx.witness.vtxinwit.resize(inputs.size()); - mtx.witness.vtxoutwit.resize(outputs.size()); return mtx; } From 34eafaf76147d361a882ac7b21361b8bfc373e6b Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Sun, 19 Sep 2021 14:52:17 +0000 Subject: [PATCH 06/11] test: sign using the PSET interface and SIGHASH_RANGEPROOF --- test/functional/feature_sighash_rangeproof.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/test/functional/feature_sighash_rangeproof.py b/test/functional/feature_sighash_rangeproof.py index 661312cae2..7f5a08f8ab 100755 --- a/test/functional/feature_sighash_rangeproof.py +++ b/test/functional/feature_sighash_rangeproof.py @@ -147,6 +147,14 @@ class SighashRangeproofTest(BitcoinTestFramework): signed_hex = self.nodes[0].signrawtransactionwithwallet(signed_hex)["hex"] test_accept = self.nodes[0].testmempoolaccept([signed_hex])[0] assert test_accept["allowed"], "not accepted: {}".format(test_accept["reject-reason"]) + + # Try signing using the PSBT interface + psbt_hex = self.nodes[0].converttopsbt(unsigned_hex) + signed_psbt = self.nodes[1].walletprocesspsbt(psbt_hex, True, "ALL|RANGEPROOF") + extracted_tx = self.nodes[0].finalizepsbt(signed_psbt["psbt"]) + assert extracted_tx["complete"] + test_accept = self.nodes[0].testmempoolaccept([extracted_tx["hex"]])[0] + assert test_accept["allowed"], "not accepted: {}".format(test_accept["reject-reason"]) else: signed_tx.rehash() From 0907fa0f6fc834b793943ecd53fd46656af486d4 Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Sun, 19 Sep 2021 20:16:37 +0000 Subject: [PATCH 07/11] test: support asset issuance in test framework sighashes Replaces #932 --- test/functional/test_framework/script.py | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/test/functional/test_framework/script.py b/test/functional/test_framework/script.py index 624d312a70..e74e9fbd8b 100644 --- a/test/functional/test_framework/script.py +++ b/test/functional/test_framework/script.py @@ -763,7 +763,16 @@ def LegacySignatureHash(script, txTo, inIdx, hashtype, enable_sighash_rangeproof # do manual sighash serialization: s = b"" s += struct.pack(" Date: Sun, 19 Sep 2021 20:48:17 +0000 Subject: [PATCH 08/11] pset: fix encoding of blinded asset issuances --- src/psbt.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/psbt.cpp b/src/psbt.cpp index b3272988e9..ebcb06e4c6 100644 --- a/src/psbt.cpp +++ b/src/psbt.cpp @@ -122,7 +122,7 @@ CMutableTransaction PartiallySignedTransaction::GetUnsignedTx(bool force_unblind txin.nSequence = input.sequence.value_or(max_sequence); txin.assetIssuance.assetBlindingNonce = input.m_issuance_blinding_nonce; txin.assetIssuance.assetEntropy = input.m_issuance_asset_entropy; - if (input.m_issuance_value != nullopt && input.m_issuance_inflation_keys_amount != nullopt && !force_unblinded) { + if (input.m_issuance_value != nullopt && input.m_issuance_inflation_keys_amount != nullopt && force_unblinded) { txin.assetIssuance.nAmount.SetToAmount(*input.m_issuance_value); txin.assetIssuance.nInflationKeys.SetToAmount(*input.m_issuance_inflation_keys_amount); } else { From d4a0b6285a86c74e8d25da68c3fb36ac564dfaab Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Sun, 19 Sep 2021 20:48:44 +0000 Subject: [PATCH 09/11] test: check asset issuances with SIGHASH_RANGEPROOF Asset issuance rangeproofs are not actually covered by SIGHASH_RANGEPROOF so this test serves more as a sanity check that our various signing mechanisms still work with issuances. Sure enough, it uncovered a PSET bug (fixed) and a blindrawtransaction bug (left a FIXME). --- test/functional/feature_sighash_rangeproof.py | 41 ++++++++++++++++--- 1 file changed, 36 insertions(+), 5 deletions(-) diff --git a/test/functional/feature_sighash_rangeproof.py b/test/functional/feature_sighash_rangeproof.py index 7f5a08f8ab..4ffdcf9ffc 100755 --- a/test/functional/feature_sighash_rangeproof.py +++ b/test/functional/feature_sighash_rangeproof.py @@ -64,7 +64,7 @@ class SighashRangeproofTest(BitcoinTestFramework): def skip_test_if_missing_module(self): self.skip_if_no_wallet() - def prepare_tx_signed_with_sighash(self, address_type, sighash_rangeproof_aware): + def prepare_tx_signed_with_sighash(self, address_type, sighash_rangeproof_aware, attach_issuance): # Create a tx that is signed with a specific version of the sighash # method. # If `sighash_rangeproof_aware` is @@ -87,6 +87,18 @@ class SighashRangeproofTest(BitcoinTestFramework): [{"txid": utxo["txid"], "vout": utxo["vout"]}], [{sink_addr: 0.9}, {"fee": 0.1}] ) + if attach_issuance: + # Attach a blinded issuance + unsigned_hex = self.nodes[1].rawissueasset( + unsigned_hex, + [{ + "asset_amount": 100, + "asset_address": self.nodes[1].getnewaddress(), + "token_amount": 100, + "token_address": self.nodes[1].getnewaddress(), + "blind": True, # FIXME: if blind=False, `blindrawtranaction` fails. Should fix this in a future PR + }] + )[0]["hex"] blinded_hex = self.nodes[1].blindrawtransaction(unsigned_hex) blinded_tx = FromHex(CTransaction(), blinded_hex) signed_hex = self.nodes[1].signrawtransactionwithwallet(blinded_hex)["hex"] @@ -132,8 +144,12 @@ class SighashRangeproofTest(BitcoinTestFramework): assert False # Make sure that the tx we manually signed is valid + signed_hex = signed_tx.serialize_with_witness().hex() test_accept = self.nodes[0].testmempoolaccept([signed_hex])[0] - assert test_accept["allowed"], "not accepted: {}".format(test_accept["reject-reason"]) + if sighash_rangeproof_aware: + assert test_accept["allowed"], "not accepted: {}".format(test_accept["reject-reason"]) + else: + assert not test_accept["allowed"], "tx was accepted" if sighash_rangeproof_aware: signed_hex = self.nodes[1].signrawtransactionwithwallet(blinded_hex, [], "ALL|RANGEPROOF")["hex"] @@ -217,7 +233,12 @@ class SighashRangeproofTest(BitcoinTestFramework): # - the tx is accepted if manually mined in a block for address_type in ADDRESS_TYPES: self.log.info("Pre-activation for {} address".format(address_type)) - tx = self.prepare_tx_signed_with_sighash(address_type, False) + tx = self.prepare_tx_signed_with_sighash(address_type, False, False) + self.assert_tx_standard(tx, False) + self.assert_tx_valid(tx, True) + + self.log.info("Pre-activation for {} address (with issuance)".format(address_type)) + tx = self.prepare_tx_signed_with_sighash(address_type, False, True) self.assert_tx_standard(tx, False) self.assert_tx_valid(tx, True) @@ -234,7 +255,12 @@ class SighashRangeproofTest(BitcoinTestFramework): # after activation. for address_type in ADDRESS_TYPES: self.log.info("Post-activation for {} address".format(address_type)) - tx = self.prepare_tx_signed_with_sighash(address_type, True) + tx = self.prepare_tx_signed_with_sighash(address_type, True, False) + self.assert_tx_standard(tx, True) + self.assert_tx_valid(tx, True) + + self.log.info("Post-activation for {} address (with issuance)".format(address_type)) + tx = self.prepare_tx_signed_with_sighash(address_type, True, True) self.assert_tx_standard(tx, True) self.assert_tx_valid(tx, True) @@ -242,7 +268,12 @@ class SighashRangeproofTest(BitcoinTestFramework): # the rangeproofs, the signature is no longer valid. for address_type in ADDRESS_TYPES: self.log.info("Post-activation invalid sighash for {} address".format(address_type)) - tx = self.prepare_tx_signed_with_sighash(address_type, False) + tx = self.prepare_tx_signed_with_sighash(address_type, False, False) + self.assert_tx_standard(tx, False) + self.assert_tx_valid(tx, False) + + self.log.info("Post-activation invalid sighash for {} address (with issuance)".format(address_type)) + tx = self.prepare_tx_signed_with_sighash(address_type, False, True) self.assert_tx_standard(tx, False) self.assert_tx_valid(tx, False) From cf4136b3b037632db41aa143a07f23b9568657fa Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Mon, 20 Sep 2021 20:46:45 +0000 Subject: [PATCH 10/11] SIGHASH_RANGEPROOF and asset issuance rangeproofs --- src/validation.cpp | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/validation.cpp b/src/validation.cpp index 4f84f79bb3..24605b7bb6 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -750,6 +750,18 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) return false; // state filled in by CheckTxInputs } + // ELEMENTS: extra policy check for consistency between issuances and their rangeproof + if (fRequireStandard) { + for (unsigned i = 0; i < std::min(tx.witness.vtxinwit.size(), tx.vin.size()); i++) { + if (!tx.vin[i].assetIssuance.nAmount.IsCommitment() && !tx.witness.vtxinwit[i].vchIssuanceAmountRangeproof.empty()) { + return state.Invalid(TxValidationResult::TX_INPUTS_NOT_STANDARD, "bad-txin-extra-issuance-rangeproof"); + } + if (!tx.vin[i].assetIssuance.nInflationKeys.IsCommitment() && !tx.witness.vtxinwit[i].vchInflationKeysRangeproof.empty()) { + return state.Invalid(TxValidationResult::TX_INPUTS_NOT_STANDARD, "bad-txin-extra-inflation-rangeproof"); + } + } + } + // Check for non-standard pay-to-script-hash in inputs const auto& params = args.m_chainparams.GetConsensus(); auto taproot_state = VersionBitsState(::ChainActive().Tip(), params, Consensus::DEPLOYMENT_TAPROOT, versionbitscache); From 71c9e90fa0fff3841e139cdf1b544eb50df5d9e7 Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Thu, 29 Jul 2021 23:48:51 +0000 Subject: [PATCH 11/11] test: add another functional test for rangeproofs in issuances --- .../feature_confidential_transactions.py | 56 +++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/test/functional/feature_confidential_transactions.py b/test/functional/feature_confidential_transactions.py index f949cbe7f6..fd4c95be08 100755 --- a/test/functional/feature_confidential_transactions.py +++ b/test/functional/feature_confidential_transactions.py @@ -16,6 +16,7 @@ from test_framework.messages import ( CTxOutValue, CTxInWitness, CTxOutWitness, + FromHex, ) from test_framework.util import ( assert_equal, @@ -102,8 +103,63 @@ class CTTest (BitcoinTestFramework): assert_equal(rec.getaddressinfo(blind_info["unconfidential"])["confidential"], blind_addr) self.nodes[0].unloadwallet("recover") + def test_null_rangeproof_enforcement(self): + self.nodes[0].generate(1) + + # 1. Produce a transaction. This is coming out of initialfreecoins so + # no signatures are needed, which slightly simplifies the test + unfunded_tx = self.nodes[0].createrawtransaction([], [{self.nodes[1].getnewaddress(): 1000}]) + unblinded_tx = self.nodes[0].fundrawtransaction(unfunded_tx)['hex'] + unsigned_tx = self.nodes[0].blindrawtransaction(unblinded_tx) + assert_equal(self.nodes[0].testmempoolaccept([unsigned_tx])[0]['allowed'], True) # tx is ok before we malleate it + tx = FromHex(CTransaction(), unsigned_tx) + assert tx.wit.vtxinwit[0].vchIssuanceAmountRangeproof == b'' + assert tx.wit.vtxinwit[0].vchInflationKeysRangeproof == b'' + + # 1a. Add an issuance with null amounts but rangeproofs + tx.wit.vtxinwit = [CTxInWitness()] + tx.wit.vtxinwit[0].vchIssuanceAmountRangeproof = b'this should not be allowed' + hex_tx = tx.serialize(with_witness=True).hex() + assert_equal(self.nodes[0].testmempoolaccept([hex_tx])[0]['allowed'], False) + + tx.wit.vtxinwit[0].vchIssuanceAmountRangeproof = b'' + tx.wit.vtxinwit[0].vchInflationKeysRangeproof = b'and neither should this' + hex_tx = tx.serialize(with_witness=True).hex() + assert_equal(self.nodes[0].testmempoolaccept([hex_tx])[0]['allowed'], False) + + # 2. Create an issuance tx with no tokens + issuance_tx = self.nodes[0].rawissueasset(unblinded_tx, [{"asset_amount": 2, "asset_address": self.nodes[1].getnewaddress()}])[0]['hex'] + issuance_tx = self.nodes[0].blindrawtransaction(issuance_tx) + assert_equal(self.nodes[0].testmempoolaccept([issuance_tx])[0]['allowed'], True) # tx is ok before we malleate it + tx = FromHex(CTransaction(), issuance_tx) + assert tx.wit.vtxinwit[0].vchIssuanceAmountRangeproof != b'' + assert tx.wit.vtxinwit[0].vchInflationKeysRangeproof == b'' + # 2a. Attach a rangeproof to the (null) reissuance token amount + tx.wit.vtxinwit[0].vchInflationKeysRangeproof = b'and this also should not be allowed' + hex_tx = tx.serialize(with_witness=True).hex() + assert_equal(self.nodes[0].testmempoolaccept([hex_tx])[0]['allowed'], False) + + # 3. Create an issuance tx with tokens but no issuance. This time we do an + # explicit issuance because we want to null out the issuance amount, and + # `rawissueasset` would want to put a confidential 0 rather than a null. + blinded_addr = self.nodes[1].getnewaddress() + unblinded_addr = self.nodes[1].validateaddress(blinded_addr)['unconfidential'] + issuance_tx = self.nodes[0].rawissueasset(unblinded_tx, [{"token_amount": 2, "token_address": unblinded_addr, "blind": False }])[0]['hex'] + issuance_tx = self.nodes[0].blindrawtransaction(issuance_tx, False, [], False) + assert_equal(self.nodes[0].testmempoolaccept([issuance_tx])[0]['allowed'], True) # tx is ok before we malleate it + tx = FromHex(CTransaction(), issuance_tx) + assert tx.wit.vtxinwit[0].vchIssuanceAmountRangeproof == b'' + assert tx.wit.vtxinwit[0].vchInflationKeysRangeproof == b'' + # 3a. Attach a rangeproof to the (null) issuance amount + tx.wit.vtxinwit[0].vchIssuanceAmountRangeproof = b'this also should not be allowed' + hex_tx = tx.serialize(with_witness=True).hex() + assert_equal(self.nodes[0].testmempoolaccept([hex_tx])[0]['allowed'], False) + def run_test(self): + print("Testing that null issuances must have null rangeproofs") + self.test_null_rangeproof_enforcement() + print("Testing wallet secret recovery") self.test_wallet_recovery()