Skip to content

Commit

Permalink
Remove redundant -datacarrier option
Browse files Browse the repository at this point in the history
  • Loading branch information
vostrnad committed Apr 23, 2024
1 parent 256e170 commit a3cd5c1
Show file tree
Hide file tree
Showing 11 changed files with 21 additions and 29 deletions.
1 change: 0 additions & 1 deletion src/init.cpp
Expand Up @@ -629,7 +629,6 @@ void SetupServerArgs(ArgsManager& argsman)
argsman.AddArg("-dustrelayfee=<amt>", strprintf("Fee rate (in %s/kvB) used to define dust, the value of an output such that it will cost more than its value in fees at this fee rate to spend it. (default: %s)", CURRENCY_UNIT, FormatMoney(DUST_RELAY_TX_FEE)), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::NODE_RELAY);
argsman.AddArg("-acceptstalefeeestimates", strprintf("Read fee estimates even if they are stale (%sdefault: %u) fee estimates are considered stale if they are %s hours old", "regtest only; ", DEFAULT_ACCEPT_STALE_FEE_ESTIMATES, Ticks<std::chrono::hours>(MAX_FILE_AGE)), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST);
argsman.AddArg("-bytespersigop", strprintf("Equivalent bytes per sigop in transactions for relay and mining (default: %u)", DEFAULT_BYTES_PER_SIGOP), ArgsManager::ALLOW_ANY, OptionsCategory::NODE_RELAY);
argsman.AddArg("-datacarrier", strprintf("Relay and mine data carrier transactions (default: %u)", DEFAULT_ACCEPT_DATACARRIER), ArgsManager::ALLOW_ANY, OptionsCategory::NODE_RELAY);
argsman.AddArg("-datacarriersize",
strprintf("Relay and mine transactions whose data-carrying raw scriptPubKey "
"is of this size or less (default: %u)",
Expand Down
3 changes: 1 addition & 2 deletions src/kernel/mempool_options.h
Expand Up @@ -50,9 +50,8 @@ struct MemPoolOptions {
* type is designated as TxoutType::NULL_DATA.
*
* Maximum size of TxoutType::NULL_DATA scripts that this node considers standard.
* If nullopt, any size is nonstandard.
*/
std::optional<unsigned> max_datacarrier_bytes{DEFAULT_ACCEPT_DATACARRIER ? std::optional{MAX_OP_RETURN_RELAY} : std::nullopt};
unsigned max_datacarrier_bytes{MAX_OP_RETURN_RELAY};
bool permit_bare_multisig{DEFAULT_PERMIT_BAREMULTISIG};
bool require_standard{true};
bool full_rbf{DEFAULT_MEMPOOL_FULL_RBF};
Expand Down
6 changes: 1 addition & 5 deletions src/node/mempool_args.cpp
Expand Up @@ -80,11 +80,7 @@ util::Result<void> ApplyArgsManOptions(const ArgsManager& argsman, const CChainP

mempool_opts.permit_bare_multisig = argsman.GetBoolArg("-permitbaremultisig", DEFAULT_PERMIT_BAREMULTISIG);

if (argsman.GetBoolArg("-datacarrier", DEFAULT_ACCEPT_DATACARRIER)) {
mempool_opts.max_datacarrier_bytes = argsman.GetIntArg("-datacarriersize", MAX_OP_RETURN_RELAY);
} else {
mempool_opts.max_datacarrier_bytes = std::nullopt;
}
mempool_opts.max_datacarrier_bytes = argsman.GetIntArg("-datacarriersize", MAX_OP_RETURN_RELAY);

mempool_opts.require_standard = !argsman.GetBoolArg("-acceptnonstdtxn", DEFAULT_ACCEPT_NON_STD_TXN);
if (!chainparams.IsTestChain() && !mempool_opts.require_standard) {
Expand Down
6 changes: 3 additions & 3 deletions src/policy/policy.cpp
Expand Up @@ -67,7 +67,7 @@ bool IsDust(const CTxOut& txout, const CFeeRate& dustRelayFeeIn)
return (txout.nValue < GetDustThreshold(txout, dustRelayFeeIn));
}

bool IsStandard(const CScript& scriptPubKey, const std::optional<unsigned>& max_datacarrier_bytes, TxoutType& whichType)
bool IsStandard(const CScript& scriptPubKey, unsigned max_datacarrier_bytes, TxoutType& whichType)
{
std::vector<std::vector<unsigned char> > vSolutions;
whichType = Solver(scriptPubKey, vSolutions);
Expand All @@ -83,15 +83,15 @@ bool IsStandard(const CScript& scriptPubKey, const std::optional<unsigned>& max_
if (m < 1 || m > n)
return false;
} else if (whichType == TxoutType::NULL_DATA) {
if (!max_datacarrier_bytes || scriptPubKey.size() > *max_datacarrier_bytes) {
if (scriptPubKey.size() > max_datacarrier_bytes) {
return false;
}
}

return true;
}

bool IsStandardTx(const CTransaction& tx, const std::optional<unsigned>& max_datacarrier_bytes, bool permit_bare_multisig, const CFeeRate& dust_relay_fee, std::string& reason)
bool IsStandardTx(const CTransaction& tx, unsigned max_datacarrier_bytes, bool permit_bare_multisig, const CFeeRate& dust_relay_fee, std::string& reason)
{
if (tx.nVersion > TX_MAX_STANDARD_VERSION || tx.nVersion < 1) {
reason = "version";
Expand Down
6 changes: 2 additions & 4 deletions src/policy/policy.h
Expand Up @@ -63,8 +63,6 @@ static constexpr unsigned int DEFAULT_ANCESTOR_SIZE_LIMIT_KVB{101};
static constexpr unsigned int DEFAULT_DESCENDANT_LIMIT{25};
/** Default for -limitdescendantsize, maximum kilobytes of in-mempool descendants */
static constexpr unsigned int DEFAULT_DESCENDANT_SIZE_LIMIT_KVB{101};
/** Default for -datacarrier */
static const bool DEFAULT_ACCEPT_DATACARRIER = true;
/**
* Default setting for -datacarriersize. 80 bytes of data, +1 for OP_RETURN,
* +2 for the pushdata opcodes.
Expand Down Expand Up @@ -125,7 +123,7 @@ CAmount GetDustThreshold(const CTxOut& txout, const CFeeRate& dustRelayFee);

bool IsDust(const CTxOut& txout, const CFeeRate& dustRelayFee);

bool IsStandard(const CScript& scriptPubKey, const std::optional<unsigned>& max_datacarrier_bytes, TxoutType& whichType);
bool IsStandard(const CScript& scriptPubKey, unsigned max_datacarrier_bytes, TxoutType& whichType);


// Changing the default transaction version requires a two step process: first
Expand All @@ -137,7 +135,7 @@ static constexpr decltype(CTransaction::nVersion) TX_MAX_STANDARD_VERSION{2};
* Check for standard transaction types
* @return True if all outputs (scriptPubKeys) use only standard transaction forms
*/
bool IsStandardTx(const CTransaction& tx, const std::optional<unsigned>& max_datacarrier_bytes, bool permit_bare_multisig, const CFeeRate& dust_relay_fee, std::string& reason);
bool IsStandardTx(const CTransaction& tx, unsigned max_datacarrier_bytes, bool permit_bare_multisig, const CFeeRate& dust_relay_fee, std::string& reason);
/**
* Check for standard transaction types
* @param[in] mapInputs Map of previous transactions that have outputs we're spending
Expand Down
4 changes: 2 additions & 2 deletions src/test/fuzz/key.cpp
Expand Up @@ -159,12 +159,12 @@ FUZZ_TARGET(key, .init = initialize_key)
assert(fillable_signing_provider_pub.HaveKey(pubkey.GetID()));

TxoutType which_type_tx_pubkey;
const bool is_standard_tx_pubkey = IsStandard(tx_pubkey_script, std::nullopt, which_type_tx_pubkey);
const bool is_standard_tx_pubkey = IsStandard(tx_pubkey_script, 0, which_type_tx_pubkey);
assert(is_standard_tx_pubkey);
assert(which_type_tx_pubkey == TxoutType::PUBKEY);

TxoutType which_type_tx_multisig;
const bool is_standard_tx_multisig = IsStandard(tx_multisig_script, std::nullopt, which_type_tx_multisig);
const bool is_standard_tx_multisig = IsStandard(tx_multisig_script, 0, which_type_tx_multisig);
assert(is_standard_tx_multisig);
assert(which_type_tx_multisig == TxoutType::MULTISIG);

Expand Down
4 changes: 2 additions & 2 deletions src/test/fuzz/transaction.cpp
Expand Up @@ -59,8 +59,8 @@ FUZZ_TARGET(transaction, .init = initialize_transaction)

const CFeeRate dust_relay_fee{DUST_RELAY_TX_FEE};
std::string reason;
const bool is_standard_with_permit_bare_multisig = IsStandardTx(tx, std::nullopt, /* permit_bare_multisig= */ true, dust_relay_fee, reason);
const bool is_standard_without_permit_bare_multisig = IsStandardTx(tx, std::nullopt, /* permit_bare_multisig= */ false, dust_relay_fee, reason);
const bool is_standard_with_permit_bare_multisig = IsStandardTx(tx, 0, /* permit_bare_multisig= */ true, dust_relay_fee, reason);
const bool is_standard_without_permit_bare_multisig = IsStandardTx(tx, 0, /* permit_bare_multisig= */ false, dust_relay_fee, reason);
if (is_standard_without_permit_bare_multisig) {
assert(is_standard_with_permit_bare_multisig);
}
Expand Down
2 changes: 1 addition & 1 deletion src/test/multisig_tests.cpp
Expand Up @@ -143,7 +143,7 @@ BOOST_AUTO_TEST_CASE(multisig_IsStandard)

const auto is_standard{[](const CScript& spk) {
TxoutType type;
bool res{::IsStandard(spk, std::nullopt, type)};
bool res{::IsStandard(spk, 0, type)};
if (res) {
BOOST_CHECK_EQUAL(type, TxoutType::MULTISIG);
}
Expand Down
6 changes: 3 additions & 3 deletions src/test/script_p2sh_tests.cpp
Expand Up @@ -20,13 +20,13 @@
// Helpers:
static bool IsStandardTx(const CTransaction& tx, bool permit_bare_multisig, std::string& reason)
{
return IsStandardTx(tx, std::nullopt, permit_bare_multisig, CFeeRate{DUST_RELAY_TX_FEE}, reason);
return IsStandardTx(tx, 0, permit_bare_multisig, CFeeRate{DUST_RELAY_TX_FEE}, reason);
}

static bool IsStandardTx(const CTransaction& tx, std::string& reason)
{
return IsStandardTx(tx, std::nullopt, /*permit_bare_multisig=*/true, CFeeRate{DUST_RELAY_TX_FEE}, reason) &&
IsStandardTx(tx, std::nullopt, /*permit_bare_multisig=*/false, CFeeRate{DUST_RELAY_TX_FEE}, reason);
return IsStandardTx(tx, 0, /*permit_bare_multisig=*/true, CFeeRate{DUST_RELAY_TX_FEE}, reason) &&
IsStandardTx(tx, 0, /*permit_bare_multisig=*/false, CFeeRate{DUST_RELAY_TX_FEE}, reason);
}

static std::vector<unsigned char> Serialize(const CScript& s)
Expand Down
2 changes: 1 addition & 1 deletion src/txmempool.h
Expand Up @@ -442,7 +442,7 @@ class CTxMemPool
const CFeeRate m_min_relay_feerate;
const CFeeRate m_dust_relay_feerate;
const bool m_permit_bare_multisig;
const std::optional<unsigned> m_max_datacarrier_bytes;
const unsigned m_max_datacarrier_bytes;
const bool m_require_standard;
const bool m_full_rbf;
const bool m_persist_v1_dat;
Expand Down
10 changes: 5 additions & 5 deletions test/functional/mempool_datacarrier.py
Expand Up @@ -24,9 +24,9 @@ def set_test_params(self):
self.num_nodes = 4
self.extra_args = [
[],
["-datacarrier=0"],
["-datacarrier=1", f"-datacarriersize={MAX_OP_RETURN_RELAY - 1}"],
["-datacarrier=1", f"-datacarriersize=2"],
["-datacarriersize=0"],
[f"-datacarriersize={MAX_OP_RETURN_RELAY - 1}"],
["-datacarriersize=2"],
]

def test_null_data_transaction(self, node: TestNode, data, success: bool) -> None:
Expand All @@ -53,13 +53,13 @@ def run_test(self):
one_byte = randbytes(1)
zero_bytes = randbytes(0)

self.log.info("Testing null data transaction with default -datacarrier and -datacarriersize values.")
self.log.info("Testing null data transaction with the default -datacarriersize value.")
self.test_null_data_transaction(node=self.nodes[0], data=default_size_data, success=True)

self.log.info("Testing a null data transaction larger than allowed by the default -datacarriersize value.")
self.test_null_data_transaction(node=self.nodes[0], data=too_long_data, success=False)

self.log.info("Testing a null data transaction with -datacarrier=false.")
self.log.info("Testing a null data transaction with -datacarriersize=0.")
self.test_null_data_transaction(node=self.nodes[1], data=default_size_data, success=False)

self.log.info("Testing a null data transaction with a size larger than accepted by -datacarriersize.")
Expand Down

0 comments on commit a3cd5c1

Please sign in to comment.