GlcoinCore/elements
1
0
Fork 0
mirror of https://github.com/ElementsProject/elements.git synced 2026-08-13 12:33:42 +02:00
Code Issues Projects Releases Packages Wiki Activity Actions 1

Merge #896: Fix logic issues with randomization of outputs in [re]issueasset_base; minor refactoring.

Browse source 
3386cae45 Make error messages distinct at each failure location. (Glenn Willen)
a4c279e29 Fix logic issues with randomization of outputs in [re]issueasset_base; minor refactoring. (Glenn Willen)

Pull request description:

  Fixes #885. This is a more comprehensive alternative to #887.

  One note: The existing code claimed to assume (and mostly did assume) that fee outputs always go at the end. Other code does not necessarily appear to assume that. I have added explicit checks for it, so it should be obvious enough if it causes problems in practice (and with a small fix, the integration tests do pass.) If we're concerned about whether the assumption is valid, I'm willing to relax the assumption (but then the old code makes even less sense.)

  @stevenroose for review? (@jhfrontz also, since #887 was yours so maybe you have opinions.)

Tree-SHA512: 512407922a574f5fcf4a7508f32b0eb9270f93acc2de561ca313bde5d737065e4af743089fcb41943e8f49700c737c84a1658ec1063847c1a086065a01a2ddfb
This commit is contained in:
Steven Roose 2020-07-27 15:56:05 +02:00
commit db9f1b9edc
No known key found for this signature in database
GPG key ID: 2F2A88D7F8D68E87
2 changed files with 59 additions and 34 deletions
Download patch file Download diff file Expand all files Collapse all files

81
src/rpc/rawtransaction.cpp
View file

@ -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<unsigned char>(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);

12
test/functional/feature_issuance.py
View file

@ -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",