Skip to content

init, doc: Replace datacarrier(size) deprecation with non-recommendation text #32714

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
achow101 wants to merge 1 commit into from
Closed

init, doc: Replace datacarrier(size) deprecation with non-recommendation text #32714

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion doc/release-notes-32406.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
- `-datacarriersize` is increased to 100,000 which effectively uncaps the limit (as the maximum transaction size limit will be hit first). It can be overridden with -datacarriersize=83 to revert to the limit enforced in previous versions. Both `-datacarrier` and `-datacarriersize` options have been marked as deprecated and are expected to be removed in a future release. (#32406)
- `-datacarriersize` is increased to 100,000 which effectively uncaps the limit (as the maximum transaction size limit will be hit first). It can be overridden with -datacarriersize=83 to revert to the limit enforced in previous versions. Both `-datacarrier` and `-datacarriersize` options are not recommended to be used, and may be removed in a future release, after a deprecation warning. (#32406)

@glozow glozow Jun 10, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do prefer "are not recommended to be used" more than "have been marked as deprecated" as it explains it more clearly to the user. However, discouragement is deprecation, it's weird to say "after a deprecation warning."


- Multiple data carrier (OP_RETURN) outputs in a transaction are now permitted for relay and mining. The `-datacarriersize` limit applies to the aggregate size of the scriptPubKeys across all such outputs in a transaction, not including the scriptPubKey size itself. (#32406)
10 changes: 3 additions & 7 deletions src/init.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -630,10 +630,10 @@ void SetupServerArgs(ArgsManager& argsman, bool can_listen_ipc)
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("(DEPRECATED) Relay and mine data carrier transactions (default: %u)", DEFAULT_ACCEPT_DATACARRIER), ArgsManager::ALLOW_ANY, OptionsCategory::NODE_RELAY);
argsman.AddArg("-datacarrier", strprintf("Relay and mine data carrier transactions. This option is not recommended for use. (default: %u)", DEFAULT_ACCEPT_DATACARRIER), ArgsManager::ALLOW_ANY, OptionsCategory::NODE_RELAY);

@sedited sedited Jun 10, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be easy enough to enumerate the potential degredations of performance here explicitly, no? Should have said this on the other PR, but the notice in general feels weirdly inconsistent to me. Basically all the other policy options have similar effects if they are tightened beyond their defaults too. What makes this one different?

@achow101 achow101 Jun 10, 2025

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, any suggested wording?

@jonatack jonatack Jun 10, 2025 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Prefer dropping the extra recommendation sentence for each of these.

Suggested change
argsman.AddArg("-datacarrier", strprintf("Relay and mine data carrier transactions. This option is not recommended for use. (default: %u)", DEFAULT_ACCEPT_DATACARRIER), 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("(DEPRECATED) Relay and mine transactions whose data-carrying raw scriptPubKeys in aggregate "
"are of this size or less, allowing multiple outputs (default: %u)",
strprintf("Relay and mine transactions whose data-carrying raw scriptPubKeys in aggregate "
"are of this size or less, allowing multiple outputs. This option is not recommended for use. (default: %u)",
MAX_OP_RETURN_RELAY),
ArgsManager::ALLOW_ANY, OptionsCategory::NODE_RELAY);
argsman.AddArg("-permitbaremultisig", strprintf("Relay transactions creating non-P2SH multisig outputs (default: %u)", DEFAULT_PERMIT_BAREMULTISIG), ArgsManager::ALLOW_ANY,
Expand Down Expand Up @@ -875,10 +875,6 @@ bool AppInitParameterInteraction(const ArgsManager& args)
InitWarning(_("Option '-checkpoints' is set but checkpoints were removed. This option has no effect."));
}

if (args.IsArgSet("-datacarriersize") || args.IsArgSet("-datacarrier")) {
InitWarning(_("Options '-datacarrier' or '-datacarriersize' are set but are marked as deprecated. They will be removed in a future version."));
}

// Error if network-specific options (-addnode, -connect, etc) are
// specified in default section of config file, but not overridden
// on the command line or in this chain's section of the config file.
Expand Down
11 changes: 0 additions & 11 deletions test/functional/mempool_datacarrier.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,17 +89,6 @@ def run_test(self):
self.test_null_data_transaction(node=self.nodes[2], data=one_byte, success=True)
self.test_null_data_transaction(node=self.nodes[3], data=one_byte, success=False)

# Clean shutdown boilerplate due to deprecation
self.expected_stderr = [
"", # node 0 has no deprecated options
"Warning: Options '-datacarrier' or '-datacarriersize' are set but are marked as deprecated. They will be removed in a future version.",
"Warning: Options '-datacarrier' or '-datacarriersize' are set but are marked as deprecated. They will be removed in a future version.",
"Warning: Options '-datacarrier' or '-datacarriersize' are set but are marked as deprecated. They will be removed in a future version.",
]

for i in range(self.num_nodes):
self.stop_node(i, expected_stderr=self.expected_stderr[i])


if __name__ == '__main__':
DataCarrierTest(__file__).main()
Loading