Skip to content

Commit 1b67848

Browse files
ximinezTapanito
authored andcommitted
refactor: Calculate numFeatures automatically (XRPLF#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`.
1 parent 368ae37 commit 1b67848

File tree

3 files changed

+86
-56
lines changed

3 files changed

+86
-56
lines changed

include/xrpl/protocol/Feature.h

Lines changed: 56 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -35,35 +35,39 @@
3535
*
3636
* Steps required to add new features to the code:
3737
*
38-
* 1) In this file, increment `numFeatures` and add a uint256 declaration
39-
* for the feature at the bottom
40-
* 2) Add a uint256 definition for the feature to the corresponding source
41-
* file (Feature.cpp). Use `registerFeature` to create the feature with
42-
* the feature's name, `Supported::no`, and `VoteBehavior::DefaultNo`. This
43-
* should be the only place the feature's name appears in code as a string.
44-
* 3) Use the uint256 as the parameter to `view.rules.enabled()` to
45-
* control flow into new code that this feature limits.
46-
* 4) If the feature development is COMPLETE, and the feature is ready to be
47-
* SUPPORTED, change the `registerFeature` parameter to Supported::yes.
48-
* 5) When the feature is ready to be ENABLED, change the `registerFeature`
49-
* parameter to `VoteBehavior::DefaultYes`.
50-
* In general, any newly supported amendments (`Supported::yes`) should have
51-
* a `VoteBehavior::DefaultNo` for at least one full release cycle. High
52-
* priority bug fixes can be an exception to this rule of thumb.
38+
* 1) Add the appropriate XRPL_FEATURE or XRPL_FIX macro definition for the
39+
* feature to features.macro with the feature's name, `Supported::no`, and
40+
* `VoteBehavior::DefaultNo`.
41+
*
42+
* 2) Use the generated variable name as the parameter to `view.rules.enabled()`
43+
* to control flow into new code that this feature limits. (featureName or
44+
* fixName)
45+
*
46+
* 3) If the feature development is COMPLETE, and the feature is ready to be
47+
* SUPPORTED, change the macro parameter in features.macro to Supported::yes.
48+
*
49+
* 4) In general, any newly supported amendments (`Supported::yes`) should have
50+
* a `VoteBehavior::DefaultNo` indefinitely so that external governance can
51+
* make the decision on when to activate it. High priority bug fixes can be
52+
* an exception to this rule. In such cases, ensure the fix has been
53+
* clearly communicated to the community using appropriate channels,
54+
* then change the macro parameter in features.macro to
55+
* `VoteBehavior::DefaultYes`. The communication process is beyond
56+
* the scope of these instructions.
57+
*
5358
*
5459
* When a feature has been enabled for several years, the conditional code
5560
* may be removed, and the feature "retired". To retire a feature:
56-
* 1) Remove the uint256 declaration from this file.
57-
* 2) MOVE the uint256 definition in Feature.cpp to the "retired features"
58-
* section at the end of the file.
59-
* 3) CHANGE the name of the variable to start with "retired".
60-
* 4) CHANGE the parameters of the `registerFeature` call to `Supported::yes`
61-
* and `VoteBehavior::DefaultNo`.
61+
*
62+
* 1) MOVE the macro definition in features.macro to the "retired features"
63+
* section at the end of the file, and change the macro to XRPL_RETIRE.
64+
*
6265
* The feature must remain registered and supported indefinitely because it
63-
* still exists in the ledger, but there is no need to vote for it because
64-
* there's nothing to vote for. If it is removed completely from the code, any
65-
* instances running that code will get amendment blocked. Removing the
66-
* feature from the ledger is beyond the scope of these instructions.
66+
* may exist in the Amendments object on ledger. There is no need to vote
67+
* for it because there's nothing to vote for. If the feature definition is
68+
* removed completely from the code, any instances running that code will get
69+
* amendment blocked. Removing the feature from the ledger is beyond the scope
70+
* of these instructions.
6771
*
6872
*/
6973

@@ -78,11 +82,32 @@ allAmendments();
7882

7983
namespace detail {
8084

85+
#pragma push_macro("XRPL_FEATURE")
86+
#undef XRPL_FEATURE
87+
#pragma push_macro("XRPL_FIX")
88+
#undef XRPL_FIX
89+
#pragma push_macro("XRPL_RETIRE")
90+
#undef XRPL_RETIRE
91+
92+
#define XRPL_FEATURE(name, supported, vote) +1
93+
#define XRPL_FIX(name, supported, vote) +1
94+
#define XRPL_RETIRE(name) +1
95+
8196
// This value SHOULD be equal to the number of amendments registered in
8297
// Feature.cpp. Because it's only used to reserve storage, and determine how
8398
// large to make the FeatureBitset, it MAY be larger. It MUST NOT be less than
8499
// the actual number of amendments. A LogicError on startup will verify this.
85-
static constexpr std::size_t numFeatures = 88;
100+
static constexpr std::size_t numFeatures =
101+
(0 +
102+
#include <xrpl/protocol/detail/features.macro>
103+
);
104+
105+
#undef XRPL_RETIRE
106+
#pragma pop_macro("XRPL_RETIRE")
107+
#undef XRPL_FIX
108+
#pragma pop_macro("XRPL_FIX")
109+
#undef XRPL_FEATURE
110+
#pragma pop_macro("XRPL_FEATURE")
86111

87112
/** Amendments that this server supports and the default voting behavior.
88113
Whether they are enabled depends on the Rules defined in the validated
@@ -322,12 +347,17 @@ foreachFeature(FeatureBitset bs, F&& f)
322347
#undef XRPL_FEATURE
323348
#pragma push_macro("XRPL_FIX")
324349
#undef XRPL_FIX
350+
#pragma push_macro("XRPL_RETIRE")
351+
#undef XRPL_RETIRE
325352

326353
#define XRPL_FEATURE(name, supported, vote) extern uint256 const feature##name;
327354
#define XRPL_FIX(name, supported, vote) extern uint256 const fix##name;
355+
#define XRPL_RETIRE(name)
328356

329357
#include <xrpl/protocol/detail/features.macro>
330358

359+
#undef XRPL_RETIRE
360+
#pragma pop_macro("XRPL_RETIRE")
331361
#undef XRPL_FIX
332362
#pragma pop_macro("XRPL_FIX")
333363
#undef XRPL_FEATURE

include/xrpl/protocol/detail/features.macro

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,9 @@
2323
#if !defined(XRPL_FIX)
2424
#error "undefined macro: XRPL_FIX"
2525
#endif
26+
#if !defined(XRPL_RETIRE)
27+
#error "undefined macro: XRPL_RETIRE"
28+
#endif
2629

2730
// Add new amendments to the top of this list.
2831
// Keep it sorted in reverse chronological order.
@@ -121,3 +124,22 @@ XRPL_FIX (NFTokenDirV1, Supported::yes, VoteBehavior::Obsolete)
121124
XRPL_FEATURE(NonFungibleTokensV1, Supported::yes, VoteBehavior::Obsolete)
122125
XRPL_FEATURE(CryptoConditionsSuite, Supported::yes, VoteBehavior::Obsolete)
123126

127+
// The following amendments have been active for at least two years. Their
128+
// pre-amendment code has been removed and the identifiers are deprecated.
129+
// All known amendments and amendments that may appear in a validated
130+
// ledger must be registered either here or above with the "active" amendments
131+
XRPL_RETIRE(MultiSign)
132+
XRPL_RETIRE(TrustSetAuth)
133+
XRPL_RETIRE(FeeEscalation)
134+
XRPL_RETIRE(PayChan)
135+
XRPL_RETIRE(CryptoConditions)
136+
XRPL_RETIRE(TickSize)
137+
XRPL_RETIRE(fix1368)
138+
XRPL_RETIRE(Escrow)
139+
XRPL_RETIRE(fix1373)
140+
XRPL_RETIRE(EnforceInvariants)
141+
XRPL_RETIRE(SortedDirectories)
142+
XRPL_RETIRE(fix1201)
143+
XRPL_RETIRE(fix1512)
144+
XRPL_RETIRE(fix1523)
145+
XRPL_RETIRE(fix1528)

src/libxrpl/protocol/Feature.cpp

Lines changed: 8 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -259,12 +259,9 @@ FeatureCollections::registerFeature(
259259
Feature const* i = getByName(name);
260260
if (!i)
261261
{
262-
// If this check fails, and you just added a feature, increase the
263-
// numFeatures value in Feature.h
264262
check(
265263
features.size() < detail::numFeatures,
266-
"More features defined than allocated. Adjust numFeatures in "
267-
"Feature.h.");
264+
"More features defined than allocated.");
268265

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

@@ -433,45 +430,26 @@ featureToName(uint256 const& f)
433430
#undef XRPL_FEATURE
434431
#pragma push_macro("XRPL_FIX")
435432
#undef XRPL_FIX
433+
#pragma push_macro("XRPL_RETIRE")
434+
#undef XRPL_RETIRE
436435

437436
#define XRPL_FEATURE(name, supported, vote) \
438437
uint256 const feature##name = registerFeature(#name, supported, vote);
439438
#define XRPL_FIX(name, supported, vote) \
440439
uint256 const fix##name = registerFeature("fix" #name, supported, vote);
440+
#define XRPL_RETIRE(name) \
441+
[[deprecated("The referenced amendment has been retired"), maybe_unused]] \
442+
uint256 const retired##name = retireFeature(#name);
441443

442444
#include <xrpl/protocol/detail/features.macro>
443445

446+
#undef XRPL_RETIRE
447+
#pragma pop_macro("XRPL_RETIRE")
444448
#undef XRPL_FIX
445449
#pragma pop_macro("XRPL_FIX")
446450
#undef XRPL_FEATURE
447451
#pragma pop_macro("XRPL_FEATURE")
448452

449-
// clang-format off
450-
451-
// The following amendments have been active for at least two years. Their
452-
// pre-amendment code has been removed and the identifiers are deprecated.
453-
// All known amendments and amendments that may appear in a validated
454-
// ledger must be registered either here or above with the "active" amendments
455-
[[deprecated("The referenced amendment has been retired"), maybe_unused]]
456-
uint256 const
457-
retiredMultiSign = retireFeature("MultiSign"),
458-
retiredTrustSetAuth = retireFeature("TrustSetAuth"),
459-
retiredFeeEscalation = retireFeature("FeeEscalation"),
460-
retiredPayChan = retireFeature("PayChan"),
461-
retiredCryptoConditions = retireFeature("CryptoConditions"),
462-
retiredTickSize = retireFeature("TickSize"),
463-
retiredFix1368 = retireFeature("fix1368"),
464-
retiredEscrow = retireFeature("Escrow"),
465-
retiredFix1373 = retireFeature("fix1373"),
466-
retiredEnforceInvariants = retireFeature("EnforceInvariants"),
467-
retiredSortedDirectories = retireFeature("SortedDirectories"),
468-
retiredFix1201 = retireFeature("fix1201"),
469-
retiredFix1512 = retireFeature("fix1512"),
470-
retiredFix1523 = retireFeature("fix1523"),
471-
retiredFix1528 = retireFeature("fix1528");
472-
473-
// clang-format on
474-
475453
// All of the features should now be registered, since variables in a cpp file
476454
// are initialized from top to bottom.
477455
//

0 commit comments

Comments
 (0)