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.

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

sfcodes harmonization for hooks #4089

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions Builds/CMake/RippledCore.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -891,6 +891,7 @@ if (tests)
src/test/protocol/InnerObjectFormats_test.cpp
src/test/protocol/Issue_test.cpp
src/test/protocol/KnownFormatToGRPC_test.cpp
src/test/protocol/Hooks_test.cpp
Copy link
Collaborator

Choose a reason for hiding this comment

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

While not critical, it would be nice if this in alphabetical order like the other entries.

src/test/protocol/PublicKey_test.cpp
src/test/protocol/Quality_test.cpp
src/test/protocol/STAccount_test.cpp
Expand Down
39 changes: 39 additions & 0 deletions src/ripple/protocol/SField.h
Original file line number Diff line number Diff line change
Expand Up @@ -341,6 +341,7 @@ extern SF_UINT8 const sfMethod;
extern SF_UINT8 const sfTransactionResult;
extern SF_UINT8 const sfTickSize;
extern SF_UINT8 const sfUNLModifyDisabling;
extern SF_UINT8 const sfHookResult;

// 16-bit integers
extern SF_UINT16 const sfLedgerEntryType;
Expand All @@ -349,6 +350,10 @@ extern SF_UINT16 const sfSignerWeight;

// 16-bit integers (uncommon)
extern SF_UINT16 const sfVersion;
extern SF_UINT16 const sfHookStateChangeCount;
extern SF_UINT16 const sfHookEmitCount;
extern SF_UINT16 const sfHookExecutionIndex;
extern SF_UINT16 const sfHookApiVersion;

// 32-bit integers (common)
extern SF_UINT32 const sfFlags;
Expand Down Expand Up @@ -392,6 +397,8 @@ extern SF_UINT32 const sfSignerListID;
extern SF_UINT32 const sfSettleDelay;
extern SF_UINT32 const sfTicketCount;
extern SF_UINT32 const sfTicketSequence;
extern SF_UINT32 const sfHookStateCount;
extern SF_UINT32 const sfEmitGeneration;

// 64-bit integers
extern SF_UINT64 const sfIndexNext;
Expand All @@ -405,6 +412,11 @@ extern SF_UINT64 const sfHighNode;
extern SF_UINT64 const sfDestinationNode;
extern SF_UINT64 const sfCookie;
extern SF_UINT64 const sfServerVersion;
extern SF_UINT64 const sfHookOn;
extern SF_UINT64 const sfHookInstructionCount;
extern SF_UINT64 const sfEmitBurden;
extern SF_UINT64 const sfHookReturnCode;
Copy link
Collaborator

Choose a reason for hiding this comment

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

There's a distinction between the common and uncommon SFields. The common ones have a slightly more efficient encoding. I had thought that we were pretty consistent about calling out which are which in this file. But on closer inspection I see that we've not been consistent.

It would be nice to add those comments into this file in those places where they are missing. But, again, that's not critical.

extern SF_UINT64 const sfReferenceCount;

// 128-bit
extern SF_HASH128 const sfEmailHash;
Expand All @@ -425,6 +437,9 @@ extern SF_HASH256 const sfLedgerIndex;
extern SF_HASH256 const sfWalletLocator;
extern SF_HASH256 const sfRootIndex;
extern SF_HASH256 const sfAccountTxnID;
extern SF_HASH256 const sfEmitParentTxnID;
extern SF_HASH256 const sfEmitNonce;
extern SF_HASH256 const sfEmitHookHash;

// 256-bit (uncommon)
extern SF_HASH256 const sfBookDirectory;
Expand All @@ -436,6 +451,10 @@ extern SF_HASH256 const sfChannel;
extern SF_HASH256 const sfConsensusHash;
extern SF_HASH256 const sfCheckID;
extern SF_HASH256 const sfValidatedHash;
extern SF_HASH256 const sfHookStateKey;
extern SF_HASH256 const sfHookHash;
extern SF_HASH256 const sfHookNamespace;
extern SF_HASH256 const sfHookSetTxnID;

// currency amount (common)
extern SF_AMOUNT const sfAmount;
Expand Down Expand Up @@ -476,6 +495,10 @@ extern SF_VL const sfMasterSignature;
extern SF_VL const sfUNLModifyValidator;
extern SF_VL const sfValidatorToDisable;
extern SF_VL const sfValidatorToReEnable;
extern SF_VL const sfHookStateData;
extern SF_VL const sfHookReturnString;
extern SF_VL const sfHookParameterName;
extern SF_VL const sfHookParameterValue;

// account
extern SF_ACCOUNT const sfAccount;
Expand All @@ -486,6 +509,10 @@ extern SF_ACCOUNT const sfAuthorize;
extern SF_ACCOUNT const sfUnauthorize;
extern SF_ACCOUNT const sfTarget;
extern SF_ACCOUNT const sfRegularKey;
extern SF_ACCOUNT const sfEmitCallback;

// account (uncommon)
extern SF_ACCOUNT const sfHookAccount;

// path set
extern SField const sfPaths;
Expand All @@ -510,6 +537,11 @@ extern SField const sfSignerEntry;
extern SField const sfSigner;
extern SField const sfMajority;
extern SField const sfDisabledValidator;
extern SField const sfEmittedTxn;
extern SField const sfHook;
extern SField const sfHookDefinition;
extern SField const sfHookParameter;
extern SField const sfHookGrant;

// array of objects
// ARRAY/1 is reserved for end of array
Expand All @@ -523,6 +555,13 @@ extern SField const sfAffectedNodes;
extern SField const sfMemos;
extern SField const sfMajorities;
extern SField const sfDisabledValidators;
extern SField const sfEmitDetails;
extern SField const sfHookExecutions;
extern SField const sfHookExecution;
extern SField const sfHookParameters;
extern SField const sfHooks;
extern SField const sfHookGrants;

//------------------------------------------------------------------------------

} // namespace ripple
Expand Down
38 changes: 38 additions & 0 deletions src/ripple/protocol/impl/SField.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ CONSTRUCT_TYPED_SFIELD(sfTransactionResult, "TransactionResult", UINT8,
// 8-bit integers (uncommon)
CONSTRUCT_TYPED_SFIELD(sfTickSize, "TickSize", UINT8, 16);
CONSTRUCT_TYPED_SFIELD(sfUNLModifyDisabling, "UNLModifyDisabling", UINT8, 17);
CONSTRUCT_TYPED_SFIELD(sfHookResult, "HookResult", UINT8, 18);

// 16-bit integers
CONSTRUCT_TYPED_SFIELD(sfLedgerEntryType, "LedgerEntryType", UINT16, 1, SField::sMD_Never);
Expand All @@ -96,6 +97,10 @@ CONSTRUCT_TYPED_SFIELD(sfSignerWeight, "SignerWeight", UINT16,

// 16-bit integers (uncommon)
CONSTRUCT_TYPED_SFIELD(sfVersion, "Version", UINT16, 16);
CONSTRUCT_TYPED_SFIELD(sfHookStateChangeCount, "HookStateChangeCount", UINT16, 17);
CONSTRUCT_TYPED_SFIELD(sfHookEmitCount, "HookEmitCount", UINT16, 18);
CONSTRUCT_TYPED_SFIELD(sfHookExecutionIndex, "HookExecutionIndex", UINT16, 19);
CONSTRUCT_TYPED_SFIELD(sfHookApiVersion, "HookApiVersion", UINT16, 20);

// 32-bit integers (common)
CONSTRUCT_TYPED_SFIELD(sfFlags, "Flags", UINT32, 2);
Expand Down Expand Up @@ -139,6 +144,8 @@ CONSTRUCT_TYPED_SFIELD(sfSignerListID, "SignerListID", UINT32,
CONSTRUCT_TYPED_SFIELD(sfSettleDelay, "SettleDelay", UINT32, 39);
CONSTRUCT_TYPED_SFIELD(sfTicketCount, "TicketCount", UINT32, 40);
CONSTRUCT_TYPED_SFIELD(sfTicketSequence, "TicketSequence", UINT32, 41);
CONSTRUCT_TYPED_SFIELD(sfHookStateCount, "HookStateCount", UINT32, 45);
Comment on lines 146 to +147
Copy link
Collaborator

Choose a reason for hiding this comment

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

There are a number of places where this commit introduces gaps in the integer codes for SFields, like this jump from 41 to 45. I get where that's coming from. I also know that closing those gaps would be disruptive to maintenance of the hooks branch.

It might be nice to call out the newly introduced gaps with comments so future pull requests (like support for XLS20) can more easily fill in those gaps.

CONSTRUCT_TYPED_SFIELD(sfEmitGeneration, "EmitGeneration", UINT32, 46);

// 64-bit integers
CONSTRUCT_TYPED_SFIELD(sfIndexNext, "IndexNext", UINT64, 1);
Expand All @@ -152,6 +159,11 @@ CONSTRUCT_TYPED_SFIELD(sfHighNode, "HighNode", UINT64,
CONSTRUCT_TYPED_SFIELD(sfDestinationNode, "DestinationNode", UINT64, 9);
CONSTRUCT_TYPED_SFIELD(sfCookie, "Cookie", UINT64, 10);
CONSTRUCT_TYPED_SFIELD(sfServerVersion, "ServerVersion", UINT64, 11);
CONSTRUCT_TYPED_SFIELD(sfEmitBurden, "EmitBurden", UINT64, 13);
CONSTRUCT_TYPED_SFIELD(sfHookOn, "HookOn", UINT64, 16);
CONSTRUCT_TYPED_SFIELD(sfHookInstructionCount, "HookInstructionCount", UINT64, 17);
CONSTRUCT_TYPED_SFIELD(sfHookReturnCode, "HookReturnCode", UINT64, 18);
CONSTRUCT_TYPED_SFIELD(sfReferenceCount, "ReferenceCount", UINT64, 19);

// 128-bit
CONSTRUCT_TYPED_SFIELD(sfEmailHash, "EmailHash", HASH128, 1);
Expand All @@ -172,6 +184,9 @@ CONSTRUCT_TYPED_SFIELD(sfLedgerIndex, "LedgerIndex", HASH256,
CONSTRUCT_TYPED_SFIELD(sfWalletLocator, "WalletLocator", HASH256, 7);
CONSTRUCT_TYPED_SFIELD(sfRootIndex, "RootIndex", HASH256, 8, SField::sMD_Always);
CONSTRUCT_TYPED_SFIELD(sfAccountTxnID, "AccountTxnID", HASH256, 9);
CONSTRUCT_TYPED_SFIELD(sfEmitParentTxnID, "EmitParentTxnID", HASH256, 11);
CONSTRUCT_TYPED_SFIELD(sfEmitNonce, "EmitNonce", HASH256, 12);
CONSTRUCT_TYPED_SFIELD(sfEmitHookHash, "EmitHookHash", HASH256, 13);

// 256-bit (uncommon)
CONSTRUCT_TYPED_SFIELD(sfBookDirectory, "BookDirectory", HASH256, 16);
Expand All @@ -184,6 +199,10 @@ CONSTRUCT_TYPED_SFIELD(sfChannel, "Channel", HASH256,
CONSTRUCT_TYPED_SFIELD(sfConsensusHash, "ConsensusHash", HASH256, 23);
CONSTRUCT_TYPED_SFIELD(sfCheckID, "CheckID", HASH256, 24);
CONSTRUCT_TYPED_SFIELD(sfValidatedHash, "ValidatedHash", HASH256, 25);
CONSTRUCT_TYPED_SFIELD(sfHookStateKey, "HookStateKey", HASH256, 30);
CONSTRUCT_TYPED_SFIELD(sfHookHash, "HookHash", HASH256, 31);
CONSTRUCT_TYPED_SFIELD(sfHookNamespace, "HookNamespace", HASH256, 32);
CONSTRUCT_TYPED_SFIELD(sfHookSetTxnID, "HookSetTxnID", HASH256, 33);

// currency amount (common)
CONSTRUCT_TYPED_SFIELD(sfAmount, "Amount", AMOUNT, 1);
Expand Down Expand Up @@ -225,6 +244,10 @@ CONSTRUCT_TYPED_SFIELD(sfMasterSignature, "MasterSignature", VL,
CONSTRUCT_TYPED_SFIELD(sfUNLModifyValidator, "UNLModifyValidator", VL, 19);
CONSTRUCT_TYPED_SFIELD(sfValidatorToDisable, "ValidatorToDisable", VL, 20);
CONSTRUCT_TYPED_SFIELD(sfValidatorToReEnable, "ValidatorToReEnable", VL, 21);
CONSTRUCT_TYPED_SFIELD(sfHookStateData, "HookStateData", VL, 22);
CONSTRUCT_TYPED_SFIELD(sfHookReturnString, "HookReturnString", VL, 23);
CONSTRUCT_TYPED_SFIELD(sfHookParameterName, "HookParameterName", VL, 24);
CONSTRUCT_TYPED_SFIELD(sfHookParameterValue, "HookParameterValue", VL, 25);

// account
CONSTRUCT_TYPED_SFIELD(sfAccount, "Account", ACCOUNT, 1);
Expand All @@ -235,6 +258,10 @@ CONSTRUCT_TYPED_SFIELD(sfAuthorize, "Authorize", ACCOUNT,
CONSTRUCT_TYPED_SFIELD(sfUnauthorize, "Unauthorize", ACCOUNT, 6);
// 7 is currently unused
CONSTRUCT_TYPED_SFIELD(sfRegularKey, "RegularKey", ACCOUNT, 8);
CONSTRUCT_TYPED_SFIELD(sfEmitCallback, "EmitCallback", ACCOUNT, 10);

// account (uncommon)
CONSTRUCT_TYPED_SFIELD(sfHookAccount, "HookAccount", ACCOUNT, 16);

// vector of 256-bit
CONSTRUCT_TYPED_SFIELD(sfIndexes, "Indexes", VECTOR256, 1, SField::sMD_Never);
Expand All @@ -256,12 +283,19 @@ CONSTRUCT_UNTYPED_SFIELD(sfNewFields, "NewFields", OBJECT,
CONSTRUCT_UNTYPED_SFIELD(sfTemplateEntry, "TemplateEntry", OBJECT, 9);
CONSTRUCT_UNTYPED_SFIELD(sfMemo, "Memo", OBJECT, 10);
CONSTRUCT_UNTYPED_SFIELD(sfSignerEntry, "SignerEntry", OBJECT, 11);
CONSTRUCT_UNTYPED_SFIELD(sfEmitDetails, "EmitDetails", OBJECT, 13);
CONSTRUCT_UNTYPED_SFIELD(sfHook, "Hook", OBJECT, 14);

// inner object (uncommon)
CONSTRUCT_UNTYPED_SFIELD(sfSigner, "Signer", OBJECT, 16);
// 17 has not been used yet
CONSTRUCT_UNTYPED_SFIELD(sfMajority, "Majority", OBJECT, 18);
CONSTRUCT_UNTYPED_SFIELD(sfDisabledValidator, "DisabledValidator", OBJECT, 19);
CONSTRUCT_UNTYPED_SFIELD(sfEmittedTxn, "EmittedTxn", OBJECT, 20);
CONSTRUCT_UNTYPED_SFIELD(sfHookExecution, "HookExecution", OBJECT, 21);
CONSTRUCT_UNTYPED_SFIELD(sfHookDefinition, "HookDefinition", OBJECT, 22);
CONSTRUCT_UNTYPED_SFIELD(sfHookParameter, "HookParameter", OBJECT, 23);
CONSTRUCT_UNTYPED_SFIELD(sfHookGrant, "HookGrant", OBJECT, 24);

// array of objects
// ARRAY/1 is reserved for end of array
Expand All @@ -273,10 +307,14 @@ CONSTRUCT_UNTYPED_SFIELD(sfNecessary, "Necessary", ARRAY,
CONSTRUCT_UNTYPED_SFIELD(sfSufficient, "Sufficient", ARRAY, 7);
CONSTRUCT_UNTYPED_SFIELD(sfAffectedNodes, "AffectedNodes", ARRAY, 8);
CONSTRUCT_UNTYPED_SFIELD(sfMemos, "Memos", ARRAY, 9);
CONSTRUCT_UNTYPED_SFIELD(sfHooks, "Hooks", ARRAY, 11);

// array of objects (uncommon)
CONSTRUCT_UNTYPED_SFIELD(sfMajorities, "Majorities", ARRAY, 16);
CONSTRUCT_UNTYPED_SFIELD(sfDisabledValidators, "DisabledValidators", ARRAY, 17);
CONSTRUCT_UNTYPED_SFIELD(sfHookExecutions, "HookExecutions", ARRAY, 18);
CONSTRUCT_UNTYPED_SFIELD(sfHookParameters, "HookParameters", ARRAY, 19);
CONSTRUCT_UNTYPED_SFIELD(sfHookGrants, "HookGrants", ARRAY, 20);

// clang-format on

Expand Down
Loading