refactor: Calculate numFeatures automatically (#5324)

Requiring manual updates of numFeatures is an annoying manual process that is easily forgotten, and leads to frequent merge conflicts. This change takes advantage of the `XRPL_FEATURE` and `XRPL_FIX` macros, and adds a new `XRPL_RETIRE` macro to automatically set `numFeatures`.
This commit is contained in:
Ed Hennis
2025-03-12 17:34:06 -04:00
committed by GitHub
parent 2406b28e64
commit ed8e32cc92
3 changed files with 86 additions and 56 deletions

View File

@@ -35,35 +35,39 @@
* *
* Steps required to add new features to the code: * Steps required to add new features to the code:
* *
* 1) In this file, increment `numFeatures` and add a uint256 declaration * 1) Add the appropriate XRPL_FEATURE or XRPL_FIX macro definition for the
* for the feature at the bottom * feature to features.macro with the feature's name, `Supported::no`, and
* 2) Add a uint256 definition for the feature to the corresponding source * `VoteBehavior::DefaultNo`.
* file (Feature.cpp). Use `registerFeature` to create the feature with *
* the feature's name, `Supported::no`, and `VoteBehavior::DefaultNo`. This * 2) Use the generated variable name as the parameter to `view.rules.enabled()`
* should be the only place the feature's name appears in code as a string. * to control flow into new code that this feature limits. (featureName or
* 3) Use the uint256 as the parameter to `view.rules.enabled()` to * fixName)
* control flow into new code that this feature limits. *
* 4) If the feature development is COMPLETE, and the feature is ready to be * 3) If the feature development is COMPLETE, and the feature is ready to be
* SUPPORTED, change the `registerFeature` parameter to Supported::yes. * SUPPORTED, change the macro parameter in features.macro to Supported::yes.
* 5) When the feature is ready to be ENABLED, change the `registerFeature` *
* parameter to `VoteBehavior::DefaultYes`. * 4) In general, any newly supported amendments (`Supported::yes`) should have
* In general, any newly supported amendments (`Supported::yes`) should have * a `VoteBehavior::DefaultNo` indefinitely so that external governance can
* a `VoteBehavior::DefaultNo` for at least one full release cycle. High * make the decision on when to activate it. High priority bug fixes can be
* priority bug fixes can be an exception to this rule of thumb. * an exception to this rule. In such cases, ensure the fix has been
* clearly communicated to the community using appropriate channels,
* then change the macro parameter in features.macro to
* `VoteBehavior::DefaultYes`. The communication process is beyond
* the scope of these instructions.
*
* *
* When a feature has been enabled for several years, the conditional code * When a feature has been enabled for several years, the conditional code
* may be removed, and the feature "retired". To retire a feature: * may be removed, and the feature "retired". To retire a feature:
* 1) Remove the uint256 declaration from this file. *
* 2) MOVE the uint256 definition in Feature.cpp to the "retired features" * 1) MOVE the macro definition in features.macro to the "retired features"
* section at the end of the file. * section at the end of the file, and change the macro to XRPL_RETIRE.
* 3) CHANGE the name of the variable to start with "retired". *
* 4) CHANGE the parameters of the `registerFeature` call to `Supported::yes`
* and `VoteBehavior::DefaultNo`.
* The feature must remain registered and supported indefinitely because it * The feature must remain registered and supported indefinitely because it
* still exists in the ledger, but there is no need to vote for it because * may exist in the Amendments object on ledger. There is no need to vote
* there's nothing to vote for. If it is removed completely from the code, any * for it because there's nothing to vote for. If the feature definition is
* instances running that code will get amendment blocked. Removing the * removed completely from the code, any instances running that code will get
* feature from the ledger is beyond the scope of these instructions. * amendment blocked. Removing the feature from the ledger is beyond the scope
* of these instructions.
* *
*/ */
@@ -78,11 +82,32 @@ allAmendments();
namespace detail { namespace detail {
#pragma push_macro("XRPL_FEATURE")
#undef XRPL_FEATURE
#pragma push_macro("XRPL_FIX")
#undef XRPL_FIX
#pragma push_macro("XRPL_RETIRE")
#undef XRPL_RETIRE
#define XRPL_FEATURE(name, supported, vote) +1
#define XRPL_FIX(name, supported, vote) +1
#define XRPL_RETIRE(name) +1
// This value SHOULD be equal to the number of amendments registered in // This value SHOULD be equal to the number of amendments registered in
// Feature.cpp. Because it's only used to reserve storage, and determine how // Feature.cpp. Because it's only used to reserve storage, and determine how
// large to make the FeatureBitset, it MAY be larger. It MUST NOT be less than // large to make the FeatureBitset, it MAY be larger. It MUST NOT be less than
// the actual number of amendments. A LogicError on startup will verify this. // the actual number of amendments. A LogicError on startup will verify this.
static constexpr std::size_t numFeatures = 88; static constexpr std::size_t numFeatures =
(0 +
#include <xrpl/protocol/detail/features.macro>
);
#undef XRPL_RETIRE
#pragma pop_macro("XRPL_RETIRE")
#undef XRPL_FIX
#pragma pop_macro("XRPL_FIX")
#undef XRPL_FEATURE
#pragma pop_macro("XRPL_FEATURE")
/** Amendments that this server supports and the default voting behavior. /** Amendments that this server supports and the default voting behavior.
Whether they are enabled depends on the Rules defined in the validated Whether they are enabled depends on the Rules defined in the validated
@@ -322,12 +347,17 @@ foreachFeature(FeatureBitset bs, F&& f)
#undef XRPL_FEATURE #undef XRPL_FEATURE
#pragma push_macro("XRPL_FIX") #pragma push_macro("XRPL_FIX")
#undef XRPL_FIX #undef XRPL_FIX
#pragma push_macro("XRPL_RETIRE")
#undef XRPL_RETIRE
#define XRPL_FEATURE(name, supported, vote) extern uint256 const feature##name; #define XRPL_FEATURE(name, supported, vote) extern uint256 const feature##name;
#define XRPL_FIX(name, supported, vote) extern uint256 const fix##name; #define XRPL_FIX(name, supported, vote) extern uint256 const fix##name;
#define XRPL_RETIRE(name)
#include <xrpl/protocol/detail/features.macro> #include <xrpl/protocol/detail/features.macro>
#undef XRPL_RETIRE
#pragma pop_macro("XRPL_RETIRE")
#undef XRPL_FIX #undef XRPL_FIX
#pragma pop_macro("XRPL_FIX") #pragma pop_macro("XRPL_FIX")
#undef XRPL_FEATURE #undef XRPL_FEATURE

View File

@@ -23,6 +23,9 @@
#if !defined(XRPL_FIX) #if !defined(XRPL_FIX)
#error "undefined macro: XRPL_FIX" #error "undefined macro: XRPL_FIX"
#endif #endif
#if !defined(XRPL_RETIRE)
#error "undefined macro: XRPL_RETIRE"
#endif
// Add new amendments to the top of this list. // Add new amendments to the top of this list.
// Keep it sorted in reverse chronological order. // Keep it sorted in reverse chronological order.
@@ -121,3 +124,22 @@ XRPL_FIX (NFTokenDirV1, Supported::yes, VoteBehavior::Obsolete)
XRPL_FEATURE(NonFungibleTokensV1, Supported::yes, VoteBehavior::Obsolete) XRPL_FEATURE(NonFungibleTokensV1, Supported::yes, VoteBehavior::Obsolete)
XRPL_FEATURE(CryptoConditionsSuite, Supported::yes, VoteBehavior::Obsolete) XRPL_FEATURE(CryptoConditionsSuite, Supported::yes, VoteBehavior::Obsolete)
// The following amendments have been active for at least two years. Their
// pre-amendment code has been removed and the identifiers are deprecated.
// All known amendments and amendments that may appear in a validated
// ledger must be registered either here or above with the "active" amendments
XRPL_RETIRE(MultiSign)
XRPL_RETIRE(TrustSetAuth)
XRPL_RETIRE(FeeEscalation)
XRPL_RETIRE(PayChan)
XRPL_RETIRE(CryptoConditions)
XRPL_RETIRE(TickSize)
XRPL_RETIRE(fix1368)
XRPL_RETIRE(Escrow)
XRPL_RETIRE(fix1373)
XRPL_RETIRE(EnforceInvariants)
XRPL_RETIRE(SortedDirectories)
XRPL_RETIRE(fix1201)
XRPL_RETIRE(fix1512)
XRPL_RETIRE(fix1523)
XRPL_RETIRE(fix1528)

View File

@@ -259,12 +259,9 @@ FeatureCollections::registerFeature(
Feature const* i = getByName(name); Feature const* i = getByName(name);
if (!i) if (!i)
{ {
// If this check fails, and you just added a feature, increase the
// numFeatures value in Feature.h
check( check(
features.size() < detail::numFeatures, features.size() < detail::numFeatures,
"More features defined than allocated. Adjust numFeatures in " "More features defined than allocated.");
"Feature.h.");
auto const f = sha512Half(Slice(name.data(), name.size())); auto const f = sha512Half(Slice(name.data(), name.size()));
@@ -433,45 +430,26 @@ featureToName(uint256 const& f)
#undef XRPL_FEATURE #undef XRPL_FEATURE
#pragma push_macro("XRPL_FIX") #pragma push_macro("XRPL_FIX")
#undef XRPL_FIX #undef XRPL_FIX
#pragma push_macro("XRPL_RETIRE")
#undef XRPL_RETIRE
#define XRPL_FEATURE(name, supported, vote) \ #define XRPL_FEATURE(name, supported, vote) \
uint256 const feature##name = registerFeature(#name, supported, vote); uint256 const feature##name = registerFeature(#name, supported, vote);
#define XRPL_FIX(name, supported, vote) \ #define XRPL_FIX(name, supported, vote) \
uint256 const fix##name = registerFeature("fix" #name, supported, vote); uint256 const fix##name = registerFeature("fix" #name, supported, vote);
#define XRPL_RETIRE(name) \
[[deprecated("The referenced amendment has been retired"), maybe_unused]] \
uint256 const retired##name = retireFeature(#name);
#include <xrpl/protocol/detail/features.macro> #include <xrpl/protocol/detail/features.macro>
#undef XRPL_RETIRE
#pragma pop_macro("XRPL_RETIRE")
#undef XRPL_FIX #undef XRPL_FIX
#pragma pop_macro("XRPL_FIX") #pragma pop_macro("XRPL_FIX")
#undef XRPL_FEATURE #undef XRPL_FEATURE
#pragma pop_macro("XRPL_FEATURE") #pragma pop_macro("XRPL_FEATURE")
// clang-format off
// The following amendments have been active for at least two years. Their
// pre-amendment code has been removed and the identifiers are deprecated.
// All known amendments and amendments that may appear in a validated
// ledger must be registered either here or above with the "active" amendments
[[deprecated("The referenced amendment has been retired"), maybe_unused]]
uint256 const
retiredMultiSign = retireFeature("MultiSign"),
retiredTrustSetAuth = retireFeature("TrustSetAuth"),
retiredFeeEscalation = retireFeature("FeeEscalation"),
retiredPayChan = retireFeature("PayChan"),
retiredCryptoConditions = retireFeature("CryptoConditions"),
retiredTickSize = retireFeature("TickSize"),
retiredFix1368 = retireFeature("fix1368"),
retiredEscrow = retireFeature("Escrow"),
retiredFix1373 = retireFeature("fix1373"),
retiredEnforceInvariants = retireFeature("EnforceInvariants"),
retiredSortedDirectories = retireFeature("SortedDirectories"),
retiredFix1201 = retireFeature("fix1201"),
retiredFix1512 = retireFeature("fix1512"),
retiredFix1523 = retireFeature("fix1523"),
retiredFix1528 = retireFeature("fix1528");
// clang-format on
// All of the features should now be registered, since variables in a cpp file // All of the features should now be registered, since variables in a cpp file
// are initialized from top to bottom. // are initialized from top to bottom.
// //