refactor: SField renames: Compute->Gas, Wasm->VM, FinishFunction->Bytecode#7696
Closed
ricky122-5 wants to merge 2 commits into
Closed
refactor: SField renames: Compute->Gas, Wasm->VM, FinishFunction->Bytecode#7696ricky122-5 wants to merge 2 commits into
ricky122-5 wants to merge 2 commits into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates XRPL Smart Escrow–related protocol fields by renaming several SField identifiers and propagating those new names through transactors, fee voting/validation plumbing, and the generated protocol_autogen transaction/ledger-entry wrappers.
Changes:
- Renames Smart Escrow
SFields (Compute→Gas, Wasm→VM, FinishFunction→Bytecode) and updates protocol macros accordingly. - Updates core implementation points (Escrow transactors, FeeVote/Change, Ledger setup, TxMeta serialization) to use the new
SFieldnames. - Updates
protocol_autogenheaders and corresponding unit tests to use the renamed builder/getter APIs and field presence checks.
Reviewed changes
Copilot reviewed 27 out of 27 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| src/xrpld/app/misc/NetworkOPs.cpp | Switches validation JSON extraction to the renamed Smart Escrow fee-limit SFields. |
| src/xrpld/app/misc/FeeVoteImpl.cpp | Updates fee voting to write/read renamed Smart Escrow fee-limit SFields. |
| src/tests/libxrpl/protocol_autogen/transactions/SetFeeTests.cpp | Updates generated transaction tests to use renamed SetFee builder/getters. |
| src/tests/libxrpl/protocol_autogen/transactions/EscrowFinishTests.cpp | Updates generated transaction tests to use Gas naming. |
| src/tests/libxrpl/protocol_autogen/transactions/EscrowCreateTests.cpp | Updates generated transaction tests to use Bytecode naming. |
| src/tests/libxrpl/protocol_autogen/ledger_entries/FeeSettingsTests.cpp | Updates generated ledger entry tests to use renamed fee-limit fields. |
| src/tests/libxrpl/protocol_autogen/ledger_entries/EscrowTests.cpp | Updates generated escrow ledger entry tests to use Bytecode naming. |
| src/test/jtx/escrow.h | Updates JTx helpers to write renamed JSON field names for escrow bytecode/gas. |
| src/test/app/PseudoTx_test.cpp | Updates pseudo-tx construction to use renamed fee-limit SFields. |
| src/test/app/FeeVote_test.cpp | Updates fee voting test transactions/expectations for renamed fee-limit SFields. |
| src/test/app/EscrowSmart_test.cpp | Updates Smart Escrow tests to use renamed bytecode/meta return-code SFields. |
| src/libxrpl/tx/transactors/system/Change.cpp | Updates SetFee/Change transactor checks and application to renamed fee-limit SFields. |
| src/libxrpl/tx/transactors/escrow/EscrowFinish.cpp | Updates EscrowFinish field handling to sfGas/sfBytecode. |
| src/libxrpl/tx/transactors/escrow/EscrowCreate.cpp | Updates EscrowCreate field handling to sfBytecode. |
| src/libxrpl/tx/transactors/escrow/EscrowCancel.cpp | Updates reserve adjustment logic to reference renamed escrow bytecode field. |
| src/libxrpl/protocol/TxMeta.cpp | Serializes VM return code using the renamed meta SField. |
| src/libxrpl/protocol/STValidation.cpp | Updates validation format to include renamed fee-limit SFields. |
| src/libxrpl/ledger/Ledger.cpp | Updates FeeSettings initialization/loading to use renamed fee-limit SFields. |
| include/xrpl/protocol/TxMeta.h | Updates TxMeta parsing to read VM return code from the renamed SField. |
| include/xrpl/protocol/detail/transactions.macro | Updates transaction field sets for EscrowCreate/EscrowFinish/SetFee. |
| include/xrpl/protocol/detail/sfields.macro | Renames the underlying SField identifiers for Smart Escrow fields. |
| include/xrpl/protocol/detail/ledger_entries.macro | Updates FeeSettings/Escrow ledger-entry field sets to renamed SFields. |
| include/xrpl/protocol_autogen/transactions/SetFee.h | Updates generated SetFee API to renamed methods/fields. |
| include/xrpl/protocol_autogen/transactions/EscrowFinish.h | Updates generated EscrowFinish API to renamed methods/fields. |
| include/xrpl/protocol_autogen/transactions/EscrowCreate.h | Updates generated EscrowCreate API to renamed methods/fields. |
| include/xrpl/protocol_autogen/ledger_entries/FeeSettings.h | Updates generated FeeSettings API to renamed methods/fields. |
| include/xrpl/protocol_autogen/ledger_entries/Escrow.h | Updates generated Escrow API to renamed methods/fields. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (!ctx.tx.isFieldPresent(sfComputationAllowance)) | ||
| if (!ctx.tx.isFieldPresent(sfGas)) | ||
| { | ||
| JLOG(ctx.j.debug()) << "FinishFunction requires ComputationAllowance"; |
Comment on lines
274
to
275
| JLOG(ctx.j.debug()) << "FinishFunction not present, " | ||
| "ComputationAllowance present"; |
Comment on lines
180
to
181
| JLOG(ctx.j.debug()) << "Must have at least one of FinishAfter, " | ||
| "Condition, or FinishFunction."; |
| if (!ctx.tx.isFieldPresent(sfFinishFunction)) | ||
| if (!ctx.tx.isFieldPresent(sfBytecode)) | ||
| { | ||
| JLOG(ctx.j.debug()) << "EscrowCreate with Data requires FinishFunction"; |
| auto const code = ctx.tx.getFieldVL(sfBytecode); | ||
| if (code.empty() || code.size() > fees.extensionSizeLimit) | ||
| { | ||
| JLOG(ctx.j.debug()) << "EscrowCreate.FinishFunction bad size " << code.size(); |
Comment on lines
+116
to
+117
| TYPED_SFIELD(sfGasLimit, UINT32, 69) | ||
| TYPED_SFIELD(sfBytecodeSizeLimit, UINT32, 70) |
Comment on lines
100
to
104
| operator()(Env&, JTx& jt) const | ||
| { | ||
| jt.jv[sfFinishFunction.jsonName] = value_; | ||
| jt.jv[sfBytecode.jsonName] = value_; | ||
| } | ||
| }; |
Comment on lines
143
to
147
| operator()(Env&, JTx& jt) const | ||
| { | ||
| jt.jv[sfComputationAllowance.jsonName] = value_; | ||
| jt.jv[sfGas.jsonName] = value_; | ||
| } | ||
| }; |
| } | ||
|
|
||
| if (auto const allowance = ctx.tx[~sfComputationAllowance]; allowance) | ||
| if (auto const allowance = ctx.tx[~sfGas]; allowance) |
| EscrowCreate::preflightSigValidated(PreflightContext const& ctx) | ||
| { | ||
| if (ctx.tx.isFieldPresent(sfFinishFunction)) | ||
| if (ctx.tx.isFieldPresent(sfBytecode)) |
ricky122-5
force-pushed
the
ripple/pr/smart-escrow-sfield-rename
branch
from
July 2, 2026 14:17
43ab511 to
4db7213
Compare
Contributor
Author
|
/ai-review |
1 similar comment
Contributor
|
/ai-review |
mvadari
reviewed
Jul 2, 2026
Comment on lines
-308
to
-309
| {sfExtensionComputeLimit, SoeOptional}, | ||
| {sfExtensionSizeLimit, SoeOptional}, |
Contributor
There was a problem hiding this comment.
These changes should be in the fees branch
ricky122-5
marked this pull request as draft
July 2, 2026 19:49
ricky122-5
marked this pull request as ready for review
July 6, 2026 19:28
4 tasks
Contributor
Author
|
Mixed in #7797 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
High Level Overview of Change
Renames three Smart Escrow SFields:
Context of Change
API Impact
libxrplchange (any change that may affectlibxrplor dependents oflibxrpl)