diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index 2025e48b94..7959a60533 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -2807,9 +2807,13 @@ struct RawIssuanceDetails }; // Appends a single issuance to the first input that doesn't have one, and includes -// a single output per asset type in shuffled positions. +// a single output per asset type in shuffled positions. Requires at least one output +// to exist (the fee output, which must be last). void issueasset_base(CMutableTransaction& mtx, RawIssuanceDetails& issuance_details, const CAmount asset_amount, const CAmount token_amount, const CTxDestination& asset_dest, const CTxDestination& token_dest, const bool blind_issuance, const uint256& contract_hash) { + assert(asset_amount > 0 || token_amount > 0); + assert(mtx.vout.size() > 0); + CScript asset_script = GetScriptForDestination(asset_dest); CScript token_script = GetScriptForDestination(token_dest); @@ -2840,13 +2844,10 @@ void issueasset_base(CMutableTransaction& mtx, RawIssuanceDetails& issuance_deta mtx.vin[issuance_input_index].assetIssuance.assetEntropy = contract_hash; - // Place assets into randomly placed output slots, just insert in place - // -1 due to fee output being at the end no matter what. - int asset_place = GetRandInt(mtx.vout.size()-1); - int token_place = GetRandInt(mtx.vout.size()); // Don't bias insertion - - assert(asset_amount > 0 || token_amount > 0); if (asset_amount > 0) { + // Fee output is required to be last. We will insert _before_ the selected position, which preserves that. + int asset_place = GetRandInt(mtx.vout.size()); + CTxOut asset_out(asset, asset_amount, asset_script); // If blinded address, insert the pubkey into the nonce field for later substitution by blinding if (IsBlindDestination(asset_dest)) { @@ -2862,6 +2863,9 @@ void issueasset_base(CMutableTransaction& mtx, RawIssuanceDetails& issuance_deta } if (token_amount > 0) { + // Calculate this _after_ we conditionally insert the asset output, which changes mtx.vout.size(). + int token_place = GetRandInt(mtx.vout.size()); + CTxOut token_out(token, token_amount, token_script); // If blinded address, insert the pubkey into the nonce field for later substitution by blinding if (IsBlindDestination(token_dest)) { @@ -2874,17 +2878,16 @@ void issueasset_base(CMutableTransaction& mtx, RawIssuanceDetails& issuance_deta } } -// Appends a single reissuance to the specified input if none exists, -// and the corresponding output in a shuffled position. Errors otherwise. -void reissueasset_base(CMutableTransaction& mtx, int& issuance_input_index, const CAmount asset_amount, const CTxDestination& asset_dest, const uint256& asset_blinder, const uint256& entropy) +// Appends a single reissuance to the specified input if none exists, and the +// corresponding output in a shuffled position. Errors otherwise. Requires at +// least one output to exist (the fee output, which must be last). +void reissueasset_base(CMutableTransaction& mtx, size_t issuance_input_index, const CAmount asset_amount, const CTxDestination& asset_dest, const uint256& asset_blinder, const uint256& entropy) { - CScript asset_script = GetScriptForDestination(asset_dest); + assert(mtx.vout.size() > 0); + assert(asset_amount > 0); + assert(mtx.vin[issuance_input_index].assetIssuance.IsNull()); - // Check if issuance already exists, error if already exists - if ((size_t)issuance_input_index >= mtx.vin.size() || !mtx.vin[issuance_input_index].assetIssuance.IsNull()) { - issuance_input_index = -1; - return; - } + CScript asset_script = GetScriptForDestination(asset_dest); CAsset asset; CalculateAsset(asset, entropy); @@ -2894,8 +2897,7 @@ void reissueasset_base(CMutableTransaction& mtx, int& issuance_input_index, cons mtx.vin[issuance_input_index].assetIssuance.nAmount = asset_amount; // Place assets into randomly placed output slots, before change output, inserted in place - assert(mtx.vout.size() >= 1); - int asset_place = GetRandInt(mtx.vout.size()-1); + int asset_place = GetRandInt(mtx.vout.size()); CTxOut asset_out(asset, asset_amount, asset_script); // If blinded address, insert the pubkey into the nonce field for later substitution by blinding @@ -2903,7 +2905,6 @@ void reissueasset_base(CMutableTransaction& mtx, int& issuance_input_index, cons CPubKey asset_blind = GetDestinationBlindingKey(asset_dest); asset_out.nNonce.vchCommitment = std::vector(asset_blind.begin(), asset_blind.end()); } - assert(asset_amount > 0); mtx.vout.insert(mtx.vout.begin()+asset_place, asset_out); mtx.vin[issuance_input_index].assetIssuance.nAmount = asset_amount; } @@ -2958,6 +2959,19 @@ UniValue rawissueasset(const JSONRPCRequest& request) // Count issuances, only append hex to final one unsigned int issuances_til_now = 0; + // Validate fee output location, required by the implementation of issueasset_base + if (mtx.vout.size() == 0){ + throw JSONRPCError(RPC_INVALID_PARAMETER, "Transaction must have at least one output."); + } + if (!mtx.vout[mtx.vout.size() - 1].IsFee()) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Last transaction output must be fee."); + } + for (size_t i = 0; i < mtx.vout.size() - 1; i++) { + if (mtx.vout[i].IsFee()) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Transaction can only have one fee output."); + } + } + for (unsigned int idx = 0; idx < issuances.size(); idx++) { const UniValue& issuance = issuances[idx]; const UniValue& issuance_o = issuance.get_obj(); @@ -3076,9 +3090,17 @@ UniValue rawreissueasset(const JSONRPCRequest& request) throw JSONRPCError(RPC_INVALID_PARAMETER, "Transaction must have at least one output."); } - UniValue issuances = request.params[1].get_array(); + // Validate fee output location, required by the implementation of reissueasset_base + if (!mtx.vout[mtx.vout.size() - 1].IsFee()) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Last transaction output must be fee."); + } + for (size_t i = 0; i < mtx.vout.size() - 1; i++) { + if (mtx.vout[i].IsFee()) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Transaction can only have one fee output."); + } + } - unsigned int num_issuances = 0; + UniValue issuances = request.params[1].get_array(); for (unsigned int idx = 0; idx < issuances.size(); idx++) { const UniValue& issuance = issuances[idx]; @@ -3104,31 +3126,24 @@ UniValue rawreissueasset(const JSONRPCRequest& request) throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Invalid asset address provided: %s", asset_address_uni.get_str())); } - int input_index = -1; + size_t input_index = -1; const UniValue& input_index_o = issuance_o["input_index"]; if (input_index_o.isNum()) { input_index = input_index_o.get_int(); if (input_index < 0) { throw JSONRPCError(RPC_INVALID_PARAMETER, "Input index must be non-negative."); + } else if (input_index >= mtx.vin.size()) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Input index must exist in transaction."); + } else if (!mtx.vin[input_index].assetIssuance.IsNull()) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Selected transaction input already has issuance data."); } } else { throw JSONRPCError(RPC_INVALID_PARAMETER, "Input indexes for all reissuances are required."); } uint256 asset_blinder = ParseHashV(issuance_o["asset_blinder"], "asset_blinder"); - uint256 entropy = ParseHashV(issuance_o["entropy"], "entropy"); - reissueasset_base(mtx, input_index, asset_amount, asset_dest, asset_blinder, entropy); - if (input_index == -1) { - throw JSONRPCError(RPC_INVALID_PARAMETER, "Selected transaction input already has issuance data."); - } - - num_issuances++; - } - - if (num_issuances != issuances.size()) { - throw JSONRPCError(RPC_INVALID_PARAMETER, "Failed to find enough blank inputs for listed issuances."); } UniValue ret(UniValue::VOBJ); diff --git a/test/functional/feature_issuance.py b/test/functional/feature_issuance.py index cb66c9a761..81c08266f8 100755 --- a/test/functional/feature_issuance.py +++ b/test/functional/feature_issuance.py @@ -290,10 +290,20 @@ class IssuanceTest(BitcoinTestFramework): # Default "blind" value is true, omitting explicit argument for last process_raw_issuance(self.nodes[0], [{"asset_amount": 1, "asset_address": nonblind_addr, "token_amount":2, "token_address":blind_addr, "blind":True}, {"asset_amount":3, "asset_address":nonblind_addr, "blind":True}, {"asset_amount":4, "asset_address":nonblind_addr, "token_amount":5, "token_address":blind_addr, "blind":True}, {"asset_amount":6, "asset_address":nonblind_addr, "token_amount":7, "token_address":blind_addr, "blind":True}, {"asset_amount":8, "asset_address":nonblind_addr, "token_amount":9, "token_address":blind_addr}]) + + # Make sure that fee is checked + valid_addr = self.nodes[0].getnewaddress() + raw_tx = self.nodes[0].createrawtransaction([], {}) + assert_raises_rpc_error(-8, "Transaction must have at least one output.", + self.nodes[0].rawissueasset, raw_tx, [{"asset_amount": 1, "asset_address": valid_addr}]) + raw_tx = self.nodes[0].createrawtransaction([], {valid_addr: Decimal("1")}) + assert_raises_rpc_error(-8, "Last transaction output must be fee.", + self.nodes[0].rawissueasset, raw_tx, [{"asset_amount": 1, "asset_address": valid_addr}]) + # Make sure that invalid addresses are rejected. valid_addr = self.nodes[0].getnewaddress() raw_tx = self.nodes[0].createrawtransaction([], {valid_addr: Decimal("1")}) - funded_tx = raw_tx #self.nodes[0].fundrawtransaction(raw_tx, {"feeRate": Decimal('0.00050000')})['hex'] + funded_tx = self.nodes[0].fundrawtransaction(raw_tx, {"feeRate": Decimal('0.00050000')})['hex'] assert_raises_rpc_error(-8, "Invalid asset address provided: foobar", self.nodes[0].rawissueasset, funded_tx, [{"asset_amount": 1, "asset_address": "foobar"}]) assert_raises_rpc_error(-8, "Invalid token address provided: foobar",