Merge bitcoin/bitcoin#33453: docs: Undeprecate datacarrier and datacarriersize configuration options

451ba9a datacarrier: Undeprecate configuration option (Anthony Towns)

Pull request description:

  Removes the deprecation for the `datacarrier` and `datacarriersize` options by reverting commit 0b4048c from bitcoin/bitcoin#32406

  **Many current Bitcoin Core users want to continue using this option**
  This statement is based on public postings from many Bitcoin Core users and not a formal survey. AJ Towns’ observation from [#32406](bitcoin/bitcoin@0b4048c#r2084024874) that “_for now there seem to be a bunch of users who like the option_” has only become more apparent in the months since.

  **The deprecation intent is unclear to users**
  This echo’s Ava Chow’s comment from #32714 that “_IMO we should not have removal warnings if there is no current plan to actually remove them._” In months since that comment, partially due to increased feedback from Bitcoin Core users wanting to keep this option, there is even less likelihood of a near term plan to remove these options. That leaves Bitcoin Core users in an unclear situation: the option could be removed in the next version or perhaps never. Removing the deprecation gives clarity for their planning purposes. Deprecating the option in the future, preferably with a removal schedule to better inform users, would still be possible.

  **Minimal downsides to removing deprecation**
  As a best practice, Bitcoin Core has avoided an option when the developers cannot articulate when they should be used. There is non-zero maintenance cost to keeping this code around (although leaving the options deprecated for a long time has the same effect). “Don’t offer users footguns” is also a good principle, but with this option, there seems to be only small impacts that can quickly be remedied by changing the option value by Bitcoin Core users. There already exist in Bitcoin Core more potentially-user-harmful options/values than what datacarrier might cause.

ACKs for top commit:
  ajtowns:
    ACK 451ba9a
  darosior:
    That said, certain users care strongly about using those options. In these conditions, i do not see the project removing the option anytime soon. Therefore i think it's technically incorrect (and confusing) to mark it as deprecated. utACK 451ba9a on removing the deprecation.
  instagibbs:
    crACK 451ba9a
  Raimo33:
    ACK 451ba9a
  Ademan:
    utACK 451ba9a
  ryanofsky:
    Code review ACK 451ba9a
  marcofleon:
    ACK 451ba9a
  achow101:
    ACK 451ba9a
  moonsettler:
    ACK bitcoin/bitcoin@451ba9a
  ismaelsadeeq:
    utACK 451ba9a 🛰️
  jonatack:
    ACK 451ba9a
  Zero-1729:
    crACK 451ba9a
  vasild:
    ACK 451ba9a

Tree-SHA512: b83fc509f5dd820976596e1ae9fb69a22ada567e0e0ac88da5fc5e940a46d8894b40cc70c3eff2cbdabd4da5ec913f0d18c1632fc906f210b308855868410699

Author: achow101

src/init.cpp

@@ -658,9 +658,9 @@ 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 (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 "
+                   strprintf("Relay and mine transactions whose data-carrying raw scriptPubKeys in aggregate "
                              "are of this size or less, allowing multiple outputs (default: %u)",
                              MAX_OP_RETURN_RELAY),
                    ArgsManager::ALLOW_ANY, OptionsCategory::NODE_RELAY);
@@ -903,10 +903,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 and are expected to be removed in a future version."));
-    }
-
     // We no longer limit the orphanage based on number of transactions but keep the option to warn users who still have it in their config.
     if (args.IsArgSet("-maxorphantx")) {
         InitWarning(_("Option '-maxorphantx' is set but no longer has any effect (see release notes). Please remove it from your configuration."));

test/functional/mempool_datacarrier.py

@@ -100,17 +100,5 @@ 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 and are expected to be removed in a future version.",
-            "Warning: Options '-datacarrier' or '-datacarriersize' are set but are marked as deprecated and are expected to be removed in a future version.",
-            "Warning: Options '-datacarrier' or '-datacarriersize' are set but are marked as deprecated and are expected to 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()