Merge ElementsProject/elements#1045: Fix SIGHASH_RANGEPROOF bugs

71c9e90fa0 test: add another functional test for rangeproofs in issuances (Andrew Poelstra)
cf4136b3b0 SIGHASH_RANGEPROOF and asset issuance rangeproofs (Andrew Poelstra)
d4a0b6285a test: check asset issuances with SIGHASH_RANGEPROOF (Andrew Poelstra)
1b23ad7093 pset: fix encoding of blinded asset issuances (Andrew Poelstra)
0907fa0f6f test: support asset issuance in test framework sighashes (Andrew Poelstra)
34eafaf761 test: sign using the PSET interface and SIGHASH_RANGEPROOF (Andrew Poelstra)
9c3c61aa0a psbt: make sure rangeproofs are present in the confidential version of the unsigned tx (Andrew Poelstra)
7d39e430c7 test: check that `signrawtransaction` does not erase RANGEPROOF signatures (Andrew Poelstra)
7a3cbda848 sign.cpp: turn on SIGHASH_SCRIPT_RANGEPROOF when checking already-existing signatures (Andrew Poelstra)
ac54a2f765 test: check that signing with the RANGEPROOF flag works (Andrew Poelstra)
ab5b376b72 SignatureHash: fix rangeproof hash for SIGHASH_RANGEPROOF (Andrew Poelstra)

Pull request description:

  Fixes #1026

  Supercedes #932

ACKs for top commit:
  achow101:
    ACK 71c9e90fa0

Tree-SHA512: ddc8074b6783344aa8b04de9e0a3d9320f70aa5f2bccd05d7e329110d861e46b2730b230a1b89ddb414dec8e9d31d04ab66e589d3f9648692fd990d03c369e81

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 {
@@ -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;
 }
 

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);

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;
     }

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);

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()
 

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"]
@@ -103,7 +115,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 +143,37 @@ class SighashRangeproofTest(BitcoinTestFramework):
         else:
             assert False
 
-        signed_tx.rehash()
+        # 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]
+        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"]
+            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"])
+
+            # 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"])
+
+            # 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()
+
         return signed_tx
 
     def assert_tx_standard(self, tx, assert_standard=True):
@@ -190,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)
 
@@ -207,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)
 
@@ -215,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)
 

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("<i", txtmp.nVersion)
-    s += ser_vector(txtmp.vin)
+    # ELEMENTS: vin serialization is different from non-witness serialization (pegin/issuance
+    #  flags are not set in the sighash)
+    s += ser_compact_size(len(txtmp.vin))
+    for i in range(len(txtmp.vin)):
+        s += txtmp.vin[i].prevout.serialize()
+        s += ser_string(txtmp.vin[i].scriptSig)
+        s += struct.pack("<I", txtmp.vin[i].nSequence)
+        if not txtmp.vin[i].assetIssuance.isNull():
+            s += txtmp.vin[i].assetIssuance.serialize()
+
     # If SIGHASH_RANGEPROOF is set, we need to add the rangeproof serialization after each output
     if enable_sighash_rangeproof and hashtype & SIGHASH_RANGEPROOF:
         s += ser_compact_size(len(txtmp.vout))
@@ -812,8 +821,11 @@ def SegwitV0SignatureHash(script, txTo, inIdx, hashtype, amount, enable_sighash_
     if not (hashtype & SIGHASH_ANYONECANPAY):
         serialize_issuance = bytes()
         # TODO actually serialize issuances
-        for _ in txTo.vin:
-            serialize_issuance += b'\x00'
+        for inp in txTo.vin:
+            if inp.assetIssuance.isNull():
+                serialize_issuance += b'\x00'
+            else:
+                serialize_issuance += inp.assetIssuance.serialize()
         hashIssuance = uint256_from_str(hash256(serialize_issuance))
 
     if ((hashtype & 0x1f) != SIGHASH_SINGLE and (hashtype & 0x1f) != SIGHASH_NONE):
@@ -848,6 +860,8 @@ def SegwitV0SignatureHash(script, txTo, inIdx, hashtype, amount, enable_sighash_
     ss += ser_string(script)
     ss += amount.serialize()
     ss += struct.pack("<I", txTo.vin[inIdx].nSequence)
+    if not txTo.vin[inIdx].assetIssuance.isNull():
+        ss += txTo.vin[inIdx].assetIssuance.serialize()
     ss += ser_uint256(hashOutputs)
     if enable_sighash_rangeproof and hashtype & SIGHASH_RANGEPROOF:
         ss += ser_uint256(hashRangeproofs)