Skip to content
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

Calculate numFeatures automatically #5324

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from
Open

Calculate numFeatures automatically #5324

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
16d1e3a
Calculate numFeatures automatically
ximinez Jan 31, 2025
a1d5340
Review feedback from @Bronek: move the macros outside of the computation
ximinez Feb 28, 2025
0fc28f1
Review feedback from @Bronek:
ximinez Feb 28, 2025
9429bf4
Update include/xrpl/protocol/Feature.h
ximinez Feb 28, 2025
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
79 changes: 53 additions & 26 deletions include/xrpl/protocol/Feature.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,35 +33,36 @@
*
* Steps required to add new features to the code:
*
* 1) In this file, increment `numFeatures` and add a uint256 declaration
* for the feature at the bottom
* 2) Add a uint256 definition for the feature to the corresponding source
* file (Feature.cpp). Use `registerFeature` to create the feature with
* the feature's name, `Supported::no`, and `VoteBehavior::DefaultNo`. This
* should be the only place the feature's name appears in code as a string.
* 3) Use the uint256 as the parameter to `view.rules.enabled()` to
* control flow into new code that this feature limits.
* 4) If the feature development is COMPLETE, and the feature is ready to be
* SUPPORTED, change the `registerFeature` parameter to Supported::yes.
* 5) When the feature is ready to be ENABLED, change the `registerFeature`
* parameter to `VoteBehavior::DefaultYes`.
* In general, any newly supported amendments (`Supported::yes`) should have
* a `VoteBehavior::DefaultNo` for at least one full release cycle. High
* priority bug fixes can be an exception to this rule of thumb.
* 1) Add the appropriate XRPL_FEATURE or XRPL_FIX macro definition for the
* feature to features.macro with the feature's name, `Supported::no`, and
* `VoteBehavior::DefaultNo`.
*
* 2) Use the generated variable name as the parameter to `view.rules.enabled()`
* to control flow into new code that this feature limits. (featureName or
* fixName)
*
* 3) If the feature development is COMPLETE, and the feature is ready to be
* SUPPORTED, change the macro parameter in features.macro to Supported::yes.
*
* 4) In general, any newly supported amendments (`Supported::yes`) should have
* a `VoteBehavior::DefaultNo` indefinitely so that external governance can
* make the decision on when to activate it. High priority bug fixes can be
* an exception to this rule. In such cases, change the macro parameter in
* features.macro to `VoteBehavior::DefaultYes`.
Copy link
Collaborator

Choose a reason for hiding this comment

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

perhaps add something like ... and ensure that the fix has been clearly communicated to the community, using the appropriate channels. . Or even swap the order i.e. ensure the communication in the first place, then change the macro parameter.

I know this goes implied, but we must assume that not everyone reading this comment will grasp the significance of clear communication in this case.

*
*
* When a feature has been enabled for several years, the conditional code
* 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"
* section at the end of the file.
* 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`.
*
* 1) MOVE the macro definition in features.macro to the "retired features"
* section at the end of the file, and change the macro to XRPL_RETIRE.
*
* 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
* there's nothing to vote for. If it is removed completely from the code, any
* instances running that code will get amendment blocked. Removing the
* feature from the ledger is beyond the scope of these instructions.
* may exist in the Amendments object on ledger. There is no need to vote
* for it because there's nothing to vote for. If the feature definition is
* removed completely from the code, any instances running that code will get
* amendment blocked. Removing the feature from the ledger is beyond the scope
* of these instructions.
*
*/

Expand All @@ -76,11 +77,32 @@ allAmendments();

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
// 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
// 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>
);
Copy link
Collaborator

Choose a reason for hiding this comment

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

love this !


#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.
Whether they are enabled depends on the Rules defined in the validated
Expand Down Expand Up @@ -320,12 +342,17 @@ foreachFeature(FeatureBitset bs, F&& f)
#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) extern uint256 const feature##name;
#define XRPL_FIX(name, supported, vote) extern uint256 const fix##name;
#define XRPL_RETIRE(name)

#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
Expand Down
22 changes: 22 additions & 0 deletions include/xrpl/protocol/detail/features.macro
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@
#if !defined(XRPL_FIX)
#error "undefined macro: XRPL_FIX"
#endif
#if !defined(XRPL_RETIRE)
#error "undefined macro: XRPL_RETIRE"
#endif

// Add new amendments to the top of this list.
// Keep it sorted in reverse chronological order.
Expand Down Expand Up @@ -121,3 +124,22 @@ XRPL_FIX (NFTokenDirV1, Supported::yes, VoteBehavior::Obsolete)
XRPL_FEATURE(NonFungibleTokensV1, 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)
38 changes: 8 additions & 30 deletions src/libxrpl/protocol/Feature.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -250,12 +250,9 @@ FeatureCollections::registerFeature(
Feature const* i = getByName(name);
if (!i)
{
// If this check fails, and you just added a feature, increase the
// numFeatures value in Feature.h
check(
features.size() < detail::numFeatures,
"More features defined than allocated. Adjust numFeatures in "
"Feature.h.");
"More features defined than allocated.");

auto const f = sha512Half(Slice(name.data(), name.size()));

Expand Down Expand Up @@ -424,45 +421,26 @@ featureToName(uint256 const& f)
#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) \
uint256 const feature##name = registerFeature(#name, supported, vote);
#define XRPL_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>

#undef XRPL_RETIRE
#pragma pop_macro("XRPL_RETIRE")
#undef XRPL_FIX
#pragma pop_macro("XRPL_FIX")
#undef 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
// are initialized from top to bottom.
//
Expand Down
Loading