From 5265bd8e4edfd192ef63d2f20bd46c03a9b4ddf7 Mon Sep 17 00:00:00 2001 From: Kassaking7 <96991820+Kassaking7@users.noreply.github.com> Date: Tue, 22 Sep 2026 17:53:46 -0400 Subject: [PATCH] line coverage for cosign-v1 (#8207) --- .github/scripts/strategy-matrix/linux.json | 2 +- .github/workflows/build-nix-images.yml | 6 +- .github/workflows/cargo-audit.yml | 2 +- .github/workflows/publish-docs.yml | 2 +- .github/workflows/reusable-clang-tidy.yml | 2 +- .github/workflows/reusable-rust.yml | 14 +- .github/workflows/reusable-upload-recipe.yml | 2 +- .pre-commit-config.yaml | 5 + bin/check-tools.sh | 1 + bin/pre-commit/check_rust_coverage_attrs.py | 149 ++++++++++++++++++ crates/Cargo.toml | 3 + crates/hello_world/Cargo.toml | 3 + crates/hello_world/src/lib.rs | 13 ++ include/xrpl/protocol/TER.h | 7 + include/xrpl/protocol/detail/features.macro | 1 + .../xrpl/protocol/detail/ledger_entries.macro | 7 + include/xrpl/protocol/detail/sfields.macro | 7 + .../xrpl/protocol/detail/transactions.macro | 9 +- .../protocol_autogen/ledger_entries/Escrow.h | 70 ++++++++ .../ledger_entries/FeeSettings.h | 105 ++++++++++++ .../transactions/EscrowCreate.h | 96 +++++++++-- .../transactions/EscrowFinish.h | 37 +++++ .../protocol_autogen/transactions/SetFee.h | 111 +++++++++++++ nix/check-tools/macos.txt | 5 +- nix/check-tools/nix-ubuntu-amd64.txt | 5 +- nix/check-tools/nix-ubuntu-arm64.txt | 5 +- src/libxrpl/protocol/STValidation.cpp | 4 + src/libxrpl/protocol/TER.cpp | 6 + src/libxrpl/tx/applySteps.cpp | 8 +- .../tx/transactors/escrow/EscrowCreate.cpp | 9 +- .../tx/transactors/escrow/EscrowFinish.cpp | 8 +- .../proposal/TransactionProposalCreate.cpp | 7 + src/libxrpl/tx/transactors/system/Change.cpp | 6 + .../app/TransactionProposalCreate_test.cpp | 67 ++++++++ src/test/rpc/LedgerEntry_test.cpp | 91 ++++++++++- .../ledger_entries/EscrowTests.cpp | 54 +++++++ .../ledger_entries/FeeSettingsTests.cpp | 81 ++++++++++ .../transactions/EscrowCreateTests.cpp | 72 +++++++-- .../transactions/EscrowFinishTests.cpp | 21 +++ .../transactions/SetFeeTests.cpp | 63 ++++++++ src/tests/libxrpl/tx/ProposalHelpers.cpp | 116 ++++++++++++++ src/xrpld/rpc/handlers/ledger/LedgerEntry.cpp | 5 + 42 files changed, 1236 insertions(+), 51 deletions(-) create mode 100755 bin/pre-commit/check_rust_coverage_attrs.py create mode 100644 src/tests/libxrpl/tx/ProposalHelpers.cpp diff --git a/.github/scripts/strategy-matrix/linux.json b/.github/scripts/strategy-matrix/linux.json index f2cddac488..2229332e41 100644 --- a/.github/scripts/strategy-matrix/linux.json +++ b/.github/scripts/strategy-matrix/linux.json @@ -1,5 +1,5 @@ { - "image_tag": "sha-473fe44", + "image_tag": "sha-060957e", "configs": { "ubuntu": [ { diff --git a/.github/workflows/build-nix-images.yml b/.github/workflows/build-nix-images.yml index e47a1f93ff..8ecf4912f5 100644 --- a/.github/workflows/build-nix-images.yml +++ b/.github/workflows/build-nix-images.yml @@ -5,15 +5,13 @@ on: branches: - develop paths: - - ".github/workflows/build-nix-images.yml" - "flake.nix" - "flake.lock" - "rust-toolchain.toml" - "nix/**" - "!nix/docker/README.md" - "!nix/devshell.nix" - - "!nix/check-tools/*.txt" - - "bin/check-tools.sh" + - "!nix/check-tools/**" - "bin/default-loader-path.sh" - "bin/install-sanitizer-libs.sh" pull_request: @@ -25,7 +23,7 @@ on: - "nix/**" - "!nix/docker/README.md" - "!nix/devshell.nix" - - "!nix/check-tools/*.txt" + - "!nix/check-tools/**" - "bin/check-tools.sh" - "bin/default-loader-path.sh" - "bin/install-sanitizer-libs.sh" diff --git a/.github/workflows/cargo-audit.yml b/.github/workflows/cargo-audit.yml index 6ddc6cdac9..6e613b822a 100644 --- a/.github/workflows/cargo-audit.yml +++ b/.github/workflows/cargo-audit.yml @@ -34,7 +34,7 @@ permissions: jobs: audit: runs-on: ubuntu-latest - container: ghcr.io/xrplf/xrpld/nix-ubuntu:sha-473fe44 + container: ghcr.io/xrplf/xrpld/nix-ubuntu:sha-060957e permissions: contents: read # Needed to open an issue on scheduled failures. diff --git a/.github/workflows/publish-docs.yml b/.github/workflows/publish-docs.yml index d4fb6faeca..735ddfc906 100644 --- a/.github/workflows/publish-docs.yml +++ b/.github/workflows/publish-docs.yml @@ -41,7 +41,7 @@ env: jobs: build: runs-on: ubuntu-latest - container: ghcr.io/xrplf/xrpld/nix-ubuntu:sha-473fe44 + container: ghcr.io/xrplf/xrpld/nix-ubuntu:sha-060957e steps: - name: Checkout repository uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 diff --git a/.github/workflows/reusable-clang-tidy.yml b/.github/workflows/reusable-clang-tidy.yml index 68ab531882..8e3b80e318 100644 --- a/.github/workflows/reusable-clang-tidy.yml +++ b/.github/workflows/reusable-clang-tidy.yml @@ -34,7 +34,7 @@ jobs: needs: [determine-files] if: ${{ needs.determine-files.outputs.cpp_changed_files != '' || needs.determine-files.outputs.need_full_run == 'true' }} runs-on: ["self-hosted", "Linux", "X64", "heavy"] - container: "ghcr.io/xrplf/xrpld/nix-debian:sha-473fe44" + container: "ghcr.io/xrplf/xrpld/nix-debian:sha-060957e" permissions: contents: read issues: write diff --git a/.github/workflows/reusable-rust.yml b/.github/workflows/reusable-rust.yml index a0199f0129..12a31aacbd 100644 --- a/.github/workflows/reusable-rust.yml +++ b/.github/workflows/reusable-rust.yml @@ -1,8 +1,9 @@ # Clippy, coverage and documentation for the Rust crates in crates/. Each runs # as an independent job on a GitHub-hosted runner, but inside the same container # image used to build the crates in the C++/Corrosion path, so the toolchain -# (and therefore the lints, coverage instrumentation and the cargo cache) matches -# what production builds use. +# (and therefore the lints and the cargo cache) matches what production builds +# use. Coverage is the exception: it needs the nightly rustc that honours +# #[coverage(off)], which the image carries alongside the pinned stable. # # Rust unit tests are deliberately NOT run here. They run as part of the C++ # build (reusable-build-test-config.yml), which already compiles the crates on a @@ -27,7 +28,7 @@ permissions: jobs: clippy: runs-on: ubuntu-latest - container: ghcr.io/xrplf/xrpld/nix-ubuntu:sha-473fe44 + container: ghcr.io/xrplf/xrpld/nix-ubuntu:sha-060957e steps: - name: Checkout repository uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 @@ -40,11 +41,14 @@ jobs: coverage: runs-on: ubuntu-latest - container: ghcr.io/xrplf/xrpld/nix-ubuntu:sha-473fe44 + container: ghcr.io/xrplf/xrpld/nix-ubuntu:sha-060957e steps: - name: Checkout repository uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + - name: Use the nightly Rust toolchain + run: rust-nightly path >>"${GITHUB_PATH}" + - name: Use cargo artifacts cache uses: ./.github/actions/cargo-cache @@ -66,7 +70,7 @@ jobs: doc: runs-on: ubuntu-latest - container: ghcr.io/xrplf/xrpld/nix-ubuntu:sha-473fe44 + container: ghcr.io/xrplf/xrpld/nix-ubuntu:sha-060957e steps: - name: Checkout repository uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 diff --git a/.github/workflows/reusable-upload-recipe.yml b/.github/workflows/reusable-upload-recipe.yml index 69de12db0e..0511458e7f 100644 --- a/.github/workflows/reusable-upload-recipe.yml +++ b/.github/workflows/reusable-upload-recipe.yml @@ -40,7 +40,7 @@ defaults: jobs: upload: runs-on: ubuntu-latest - container: ghcr.io/xrplf/xrpld/nix-ubuntu:sha-473fe44 + container: ghcr.io/xrplf/xrpld/nix-ubuntu:sha-060957e env: REMOTE_NAME: ${{ inputs.remote_name }} CONAN_LOGIN_USERNAME_XRPLF: ${{ secrets.remote_username }} diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index f223ab1684..8721ad7863 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -70,6 +70,11 @@ repos: language: system types: [rust] pass_filenames: false # rustfmt formats the whole workspace + - id: check-coverage-attrs + name: check Rust coverage attributes + entry: ./bin/pre-commit/check_rust_coverage_attrs.py + language: python + files: ^crates/.*\.rs$ - repo: https://github.com/BlankSpruce/gersemi-pre-commit rev: e98930bdc210d3387007f9252d8c1694ea7e410f # frozen: 0.27.7 diff --git a/bin/check-tools.sh b/bin/check-tools.sh index 8273375428..ed76861aa4 100755 --- a/bin/check-tools.sh +++ b/bin/check-tools.sh @@ -158,6 +158,7 @@ if [ "${os}" = "linux" ] || [ "${os}" = "macos" ]; then check cargo-nextest cargo nextest --version check clippy-driver check rust-analyzer + check rust-nightly rust-nightly run rustc --version check rustc check rustfmt fi diff --git a/bin/pre-commit/check_rust_coverage_attrs.py b/bin/pre-commit/check_rust_coverage_attrs.py new file mode 100755 index 0000000000..dba56d5756 --- /dev/null +++ b/bin/pre-commit/check_rust_coverage_attrs.py @@ -0,0 +1,149 @@ +#!/usr/bin/env python3 +""" +Check that Rust unit tests stay out of the coverage report. + +cargo-llvm-cov instruments the test code along with everything else, so a test +module that is not excluded counts its own body as covered and inflates the +reported number. Excluding it takes two attributes: + + * every `#[cfg(test)]` module carries + `#[cfg_attr(coverage_nightly, coverage(off))]`; + * every crate root (lib.rs, main.rs) carries + `#![cfg_attr(coverage_nightly, feature(coverage_attribute))]`, which the + attribute above needs in order to compile. + +Both are inert outside the coverage job: cargo-llvm-cov defines +`coverage_nightly` only when it runs on a nightly toolchain. + +The crate-root gate is checked even in a crate that has no tests yet, because +that is what lets the first test module added later carry the attribute without +a build failure. Missing it is a hard error, so it cannot go unnoticed; a +missing `coverage(off)` fails open, which is why this check exists. + +Matching is on exact attribute text, which works because `cargo fmt` runs over +the whole workspace in the hook ahead of this one: rustfmt puts every attribute +on its own line and normalizes what is inside it, turning `#[cfg( test )]` +and `#[cfg(test,)]` alike into `#[cfg(test)]`. So there is nothing here that +parses Rust. The price is that a cfg this file does not spell out literally -- +`all(test, ...)`, `any(test, ...)`, `not(test)` -- is reported rather than +classified, on the grounds that guessing at coverage semantics is how a check +like this ends up quietly wrong. + +Usage: ./bin/pre-commit/check_rust_coverage_attrs.py ... + +Exit status is non-zero if any violation is found. +""" + +import re +import sys +from dataclasses import dataclass +from pathlib import Path + +CRATE_ROOTS = {"lib.rs", "main.rs"} + +FEATURE_ATTR = "#![cfg_attr(coverage_nightly, feature(coverage_attribute))]" +COVERAGE_OFF_ATTR = "#[cfg_attr(coverage_nightly, coverage(off))]" +CFG_TEST_ATTR = "#[cfg(test)]" + +# Any other cfg that mentions `test`. String literals are blanked before this +# runs, so `feature = "test"` does not read as the `test` cfg. +RE_CFG_MENTIONS_TEST = re.compile(r"^#\[cfg\(.*\btest\b.*\)\]$") +RE_STRING = re.compile(r'"(?:[^"\\]|\\.)*"') +RE_MOD = re.compile(r"^(?:pub(?:\([^)]*\))?\s+)?mod\s+([A-Za-z_]\w*)") + + +@dataclass(frozen=True) +class Finding: + line: int + label: str + message: str + + +def _check_module(attrs: list[str], line: int, name: str) -> list[Finding]: + """Findings for one module, given the attributes attached to it.""" + if COVERAGE_OFF_ATTR in attrs: + return [] # excluded from coverage; which cfg gates it does not matter + if CFG_TEST_ATTR in attrs: + return [ + Finding( + line, + "missing-coverage-off", + f"`mod {name}` is #[cfg(test)] but not excluded from coverage; " + f"add {COVERAGE_OFF_ATTR}", + ) + ] + unclassified = [ + attr for attr in attrs if RE_CFG_MENTIONS_TEST.match(RE_STRING.sub('""', attr)) + ] + if unclassified: + return [ + Finding( + line, + "unclassified-cfg", + f"`mod {name}` is gated on {unclassified[0]}, which this check " + f"cannot tell apart from a module that ships in the library; " + f"add {COVERAGE_OFF_ATTR} if it is test-only, or teach this " + f"check the cfg if it is not", + ) + ] + return [] + + +def _check_test_modules(lines: list[str]) -> list[Finding]: + """Findings for every test module that is not excluded from coverage.""" + findings: list[Finding] = [] + attrs: list[str] = [] + attrs_line = 0 + for number, raw in enumerate(lines, start=1): + stripped = raw.strip() + # Blank lines and comments are allowed between an attribute and its item. + if not stripped or stripped.startswith("//"): + continue + if stripped.startswith("#["): + if not attrs: + attrs_line = number + attrs.append(stripped) + continue + module = RE_MOD.match(stripped) + if module is not None and attrs: + findings += _check_module(attrs, attrs_line, module.group(1)) + attrs = [] + return findings + + +def _check_crate_root(name: str, lines: list[str]) -> list[Finding]: + """A finding if a crate root is missing the coverage_attribute feature gate.""" + if name not in CRATE_ROOTS: + return [] + if any(line.strip() == FEATURE_ATTR for line in lines): + return [] + return [ + Finding( + 1, + "missing-feature-gate", + f"crate root is missing {FEATURE_ATTR}", + ) + ] + + +def check_source(name: str, text: str) -> list[Finding]: + """Findings for one file's contents; `name` is its base name (lib.rs, ...).""" + lines = text.splitlines() + return _check_crate_root(name, lines) + _check_test_modules(lines) + + +def check_file(path: Path) -> list[Finding]: + return check_source(path.name, path.read_text(encoding="utf-8")) + + +def main() -> int: + total = 0 + for path in (Path(name) for name in sys.argv[1:]): + for finding in check_file(path): + total += 1 + print(f"{path}:{finding.line}: {finding.label}: {finding.message}") + return 1 if total else 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/crates/Cargo.toml b/crates/Cargo.toml index 0bb0e9c550..36c79f1752 100644 --- a/crates/Cargo.toml +++ b/crates/Cargo.toml @@ -8,6 +8,9 @@ cxx = { version = "1.0.198", features = ["c++20"] } [workspace.package] edition = "2024" +[workspace.lints.rust] +unexpected_cfgs = { level = "warn", check-cfg = [ 'cfg(coverage)', 'cfg(coverage_nightly)' ] } + [profile.release] opt-level = 3 overflow-checks = true diff --git a/crates/hello_world/Cargo.toml b/crates/hello_world/Cargo.toml index 2e5a329c9a..50f02cc95f 100644 --- a/crates/hello_world/Cargo.toml +++ b/crates/hello_world/Cargo.toml @@ -8,3 +8,6 @@ crate-type = ["staticlib"] [dependencies] cxx.workspace = true + +[lints] +workspace = true diff --git a/crates/hello_world/src/lib.rs b/crates/hello_world/src/lib.rs index b1cb121fa0..6f69801a1d 100644 --- a/crates/hello_world/src/lib.rs +++ b/crates/hello_world/src/lib.rs @@ -1,3 +1,5 @@ +#![cfg_attr(coverage_nightly, feature(coverage_attribute))] + #[cxx::bridge(namespace = "rs::hello_world")] mod ffi { extern "Rust" { @@ -8,3 +10,14 @@ mod ffi { pub fn hello_world() -> String { "hello_world".to_string() } + +#[cfg(test)] +#[cfg_attr(coverage_nightly, coverage(off))] +mod tests { + use super::*; + + #[test] + fn hello_world_returns_hello_world() { + assert_eq!(hello_world(), "hello_world") + } +} diff --git a/include/xrpl/protocol/TER.h b/include/xrpl/protocol/TER.h index 730d021254..5702b01d1d 100644 --- a/include/xrpl/protocol/TER.h +++ b/include/xrpl/protocol/TER.h @@ -129,8 +129,11 @@ enum TEMcodes : TERUnderlyingType { temARRAY_TOO_LARGE, temBAD_TRANSFER_FEE, temINVALID_INNER_BATCH, + temBAD_MPT, temBAD_CIPHERTEXT, + temINVALID_BYTECODE, + temTEMP_DISABLED, }; //------------------------------------------------------------------------------ @@ -179,6 +182,8 @@ enum TEFcodes : TERUnderlyingType { tefINVALID_LEDGER_FIX_TYPE, tefNO_DST_PARTIAL, tefBAD_PATH_COUNT, + tefNO_BYTECODE, + tefBYTECODE_NOT_INCLUDED, }; //------------------------------------------------------------------------------ @@ -370,6 +375,8 @@ enum TECcodes : TERUnderlyingType { tecNO_DELEGATE_PERMISSION = 198, tecBAD_PROOF = 199, tecNO_SPONSOR_PERMISSION = 200, + tecOUT_OF_GAS = 201, + tecBYTECODE_REJECTED = 202, }; //------------------------------------------------------------------------------ diff --git a/include/xrpl/protocol/detail/features.macro b/include/xrpl/protocol/detail/features.macro index 4f2023448b..eb92107133 100644 --- a/include/xrpl/protocol/detail/features.macro +++ b/include/xrpl/protocol/detail/features.macro @@ -16,6 +16,7 @@ // Keep it sorted in reverse chronological order. XRPL_FEATURE(Cosign, Supported::No, VoteBehavior::DefaultNo) +XRPL_FEATURE(SmartEscrow, Supported::No, VoteBehavior::DefaultNo) XRPL_FEATURE(LendingProtocolV1_2, Supported::No, VoteBehavior::DefaultNo) XRPL_FIX (Cleanup3_5_0, Supported::Yes, VoteBehavior::DefaultNo) XRPL_FEATURE(ConfidentialMPTKeyRotation, Supported::No, VoteBehavior::DefaultNo) diff --git a/include/xrpl/protocol/detail/ledger_entries.macro b/include/xrpl/protocol/detail/ledger_entries.macro index 459475f9d0..1675d05e2d 100644 --- a/include/xrpl/protocol/detail/ledger_entries.macro +++ b/include/xrpl/protocol/detail/ledger_entries.macro @@ -309,6 +309,11 @@ LEDGER_ENTRY(ltFEE_SETTINGS, 0x0073, FeeSettings, fee, ({ {sfBaseFeeDrops, SoeOptional}, {sfReserveBaseDrops, SoeOptional}, {sfReserveIncrementDrops, SoeOptional}, + // Smart Escrow fields + {sfGasLimit, SoeOptional}, + {sfBytecodeSizeLimit, SoeOptional}, + {sfGasPrice, SoeOptional}, + {sfPreviousTxnID, SoeOptional}, {sfPreviousTxnLgrSeq, SoeOptional}, })) @@ -339,6 +344,8 @@ LEDGER_ENTRY(ltESCROW, 0x0075, Escrow, escrow, ({ {sfCondition, SoeOptional}, {sfCancelAfter, SoeOptional}, {sfFinishAfter, SoeOptional}, + {sfBytecode, SoeOptional}, + {sfData, SoeOptional}, {sfSourceTag, SoeOptional}, {sfDestinationTag, SoeOptional}, {sfOwnerNode, SoeRequired}, diff --git a/include/xrpl/protocol/detail/sfields.macro b/include/xrpl/protocol/detail/sfields.macro index c06f6f2498..14c86c9f79 100644 --- a/include/xrpl/protocol/detail/sfields.macro +++ b/include/xrpl/protocol/detail/sfields.macro @@ -123,6 +123,11 @@ TYPED_SFIELD(sfIssuerKeyEpoch, UINT32, 77) TYPED_SFIELD(sfAuditorKeyEpoch, UINT32, 78) TYPED_SFIELD(sfIssuerKeyMirrorEpoch, UINT32, 79) TYPED_SFIELD(sfAuditorKeyMirrorEpoch, UINT32, 80) +TYPED_SFIELD(sfGasLimit, UINT32, 81) +TYPED_SFIELD(sfBytecodeSizeLimit, UINT32, 82) +TYPED_SFIELD(sfGasPrice, UINT32, 83) +TYPED_SFIELD(sfGas, UINT32, 84) +TYPED_SFIELD(sfGasUsed, UINT32, 85) // 64-bit integers (common) TYPED_SFIELD(sfIndexNext, UINT64, 1) @@ -239,6 +244,7 @@ TYPED_SFIELD(sfManagementFeeOutstanding, NUMBER, 17, SField::kSmdNeedsAsset // 32-bit signed (common) TYPED_SFIELD(sfLoanScale, INT32, 1) TYPED_SFIELD(sfRemainingOwnerCountDelta, INT32, 2) +TYPED_SFIELD(sfVMReturnCode, INT32, 3) // currency amount (common) TYPED_SFIELD(sfAmount, AMOUNT, 1) @@ -323,6 +329,7 @@ TYPED_SFIELD(sfAuditorEncryptedAmount, VL, 43) TYPED_SFIELD(sfAuditorEncryptionKey, VL, 44) TYPED_SFIELD(sfAmountCommitment, VL, 45) TYPED_SFIELD(sfBalanceCommitment, VL, 46) +TYPED_SFIELD(sfBytecode, VL, 47) // account (common) TYPED_SFIELD(sfAccount, ACCOUNT, 1) diff --git a/include/xrpl/protocol/detail/transactions.macro b/include/xrpl/protocol/detail/transactions.macro index 0ad99cb985..55ab63f1f7 100644 --- a/include/xrpl/protocol/detail/transactions.macro +++ b/include/xrpl/protocol/detail/transactions.macro @@ -66,11 +66,13 @@ TRANSACTION(ttPAYMENT, 0, Payment, #endif TRANSACTION(ttESCROW_CREATE, 1, EscrowCreate, ({.delegable = Delegation::Delegable}), ({ {sfDestination, SoeRequired}, + {sfDestinationTag, SoeOptional}, {sfAmount, SoeRequired, SoeMptSupported}, {sfCondition, SoeOptional}, {sfCancelAfter, SoeOptional}, {sfFinishAfter, SoeOptional}, - {sfDestinationTag, SoeOptional}, + {sfBytecode, SoeOptional}, + {sfData, SoeOptional}, })) /** This transaction type completes an existing escrow. */ @@ -83,6 +85,7 @@ TRANSACTION(ttESCROW_FINISH, 2, EscrowFinish, ({.delegable = Delegation::Delegab {sfFulfillment, SoeOptional}, {sfCondition, SoeOptional}, {sfCredentialIDs, SoeOptional}, + {sfGas, SoeOptional}, })) @@ -1173,6 +1176,10 @@ TRANSACTION(ttFEE, 101, SetFee, {sfBaseFeeDrops, SoeOptional}, {sfReserveBaseDrops, SoeOptional}, {sfReserveIncrementDrops, SoeOptional}, + // Smart Escrow fields + {sfGasLimit, SoeOptional}, + {sfBytecodeSizeLimit, SoeOptional}, + {sfGasPrice, SoeOptional}, })) /** This system-generated transaction type is used to update the network's negative UNL diff --git a/include/xrpl/protocol_autogen/ledger_entries/Escrow.h b/include/xrpl/protocol_autogen/ledger_entries/Escrow.h index 106d69722f..fc01c8b858 100644 --- a/include/xrpl/protocol_autogen/ledger_entries/Escrow.h +++ b/include/xrpl/protocol_autogen/ledger_entries/Escrow.h @@ -174,6 +174,54 @@ public: return this->sle_->isFieldPresent(sfFinishAfter); } + /** + * @brief Get sfBytecode (SoeOptional) + * @return The field value, or std::nullopt if not present. + */ + [[nodiscard]] + protocol_autogen::Optional + getBytecode() const + { + if (hasBytecode()) + return this->sle_->at(sfBytecode); + return std::nullopt; + } + + /** + * @brief Check if sfBytecode is present. + * @return True if the field is present, false otherwise. + */ + [[nodiscard]] + bool + hasBytecode() const + { + return this->sle_->isFieldPresent(sfBytecode); + } + + /** + * @brief Get sfData (SoeOptional) + * @return The field value, or std::nullopt if not present. + */ + [[nodiscard]] + protocol_autogen::Optional + getData() const + { + if (hasData()) + return this->sle_->at(sfData); + return std::nullopt; + } + + /** + * @brief Check if sfData is present. + * @return True if the field is present, false otherwise. + */ + [[nodiscard]] + bool + hasData() const + { + return this->sle_->isFieldPresent(sfData); + } + /** * @brief Get sfSourceTag (SoeOptional) * @return The field value, or std::nullopt if not present. @@ -453,6 +501,28 @@ public: return *this; } + /** + * @brief Set sfBytecode (SoeOptional) + * @return Reference to this builder for method chaining. + */ + EscrowBuilder& + setBytecode(std::decay_t const& value) + { + object_[sfBytecode] = value; + return *this; + } + + /** + * @brief Set sfData (SoeOptional) + * @return Reference to this builder for method chaining. + */ + EscrowBuilder& + setData(std::decay_t const& value) + { + object_[sfData] = value; + return *this; + } + /** * @brief Set sfSourceTag (SoeOptional) * @return Reference to this builder for method chaining. diff --git a/include/xrpl/protocol_autogen/ledger_entries/FeeSettings.h b/include/xrpl/protocol_autogen/ledger_entries/FeeSettings.h index 21478f749d..826ad6983e 100644 --- a/include/xrpl/protocol_autogen/ledger_entries/FeeSettings.h +++ b/include/xrpl/protocol_autogen/ledger_entries/FeeSettings.h @@ -213,6 +213,78 @@ public: return this->sle_->isFieldPresent(sfReserveIncrementDrops); } + /** + * @brief Get sfGasLimit (SoeOptional) + * @return The field value, or std::nullopt if not present. + */ + [[nodiscard]] + protocol_autogen::Optional + getGasLimit() const + { + if (hasGasLimit()) + return this->sle_->at(sfGasLimit); + return std::nullopt; + } + + /** + * @brief Check if sfGasLimit is present. + * @return True if the field is present, false otherwise. + */ + [[nodiscard]] + bool + hasGasLimit() const + { + return this->sle_->isFieldPresent(sfGasLimit); + } + + /** + * @brief Get sfBytecodeSizeLimit (SoeOptional) + * @return The field value, or std::nullopt if not present. + */ + [[nodiscard]] + protocol_autogen::Optional + getBytecodeSizeLimit() const + { + if (hasBytecodeSizeLimit()) + return this->sle_->at(sfBytecodeSizeLimit); + return std::nullopt; + } + + /** + * @brief Check if sfBytecodeSizeLimit is present. + * @return True if the field is present, false otherwise. + */ + [[nodiscard]] + bool + hasBytecodeSizeLimit() const + { + return this->sle_->isFieldPresent(sfBytecodeSizeLimit); + } + + /** + * @brief Get sfGasPrice (SoeOptional) + * @return The field value, or std::nullopt if not present. + */ + [[nodiscard]] + protocol_autogen::Optional + getGasPrice() const + { + if (hasGasPrice()) + return this->sle_->at(sfGasPrice); + return std::nullopt; + } + + /** + * @brief Check if sfGasPrice is present. + * @return True if the field is present, false otherwise. + */ + [[nodiscard]] + bool + hasGasPrice() const + { + return this->sle_->isFieldPresent(sfGasPrice); + } + /** * @brief Get sfPreviousTxnID (SoeOptional) * @return The field value, or std::nullopt if not present. @@ -375,6 +447,39 @@ public: return *this; } + /** + * @brief Set sfGasLimit (SoeOptional) + * @return Reference to this builder for method chaining. + */ + FeeSettingsBuilder& + setGasLimit(std::decay_t const& value) + { + object_[sfGasLimit] = value; + return *this; + } + + /** + * @brief Set sfBytecodeSizeLimit (SoeOptional) + * @return Reference to this builder for method chaining. + */ + FeeSettingsBuilder& + setBytecodeSizeLimit(std::decay_t const& value) + { + object_[sfBytecodeSizeLimit] = value; + return *this; + } + + /** + * @brief Set sfGasPrice (SoeOptional) + * @return Reference to this builder for method chaining. + */ + FeeSettingsBuilder& + setGasPrice(std::decay_t const& value) + { + object_[sfGasPrice] = value; + return *this; + } + /** * @brief Set sfPreviousTxnID (SoeOptional) * @return Reference to this builder for method chaining. diff --git a/include/xrpl/protocol_autogen/transactions/EscrowCreate.h b/include/xrpl/protocol_autogen/transactions/EscrowCreate.h index 3d28a12cee..78f9f00033 100644 --- a/include/xrpl/protocol_autogen/transactions/EscrowCreate.h +++ b/include/xrpl/protocol_autogen/transactions/EscrowCreate.h @@ -58,6 +58,32 @@ public: return this->tx_->at(sfDestination); } + /** + * @brief Get sfDestinationTag (SoeOptional) + * @return The field value, or std::nullopt if not present. + */ + [[nodiscard]] + protocol_autogen::Optional + getDestinationTag() const + { + if (hasDestinationTag()) + { + return this->tx_->at(sfDestinationTag); + } + return std::nullopt; + } + + /** + * @brief Check if sfDestinationTag is present. + * @return True if the field is present, false otherwise. + */ + [[nodiscard]] + bool + hasDestinationTag() const + { + return this->tx_->isFieldPresent(sfDestinationTag); + } + /** * @brief Get sfAmount (SoeRequired) * @note This field supports MPT (Multi-Purpose Token) amounts. @@ -149,29 +175,55 @@ public: } /** - * @brief Get sfDestinationTag (SoeOptional) + * @brief Get sfBytecode (SoeOptional) * @return The field value, or std::nullopt if not present. */ [[nodiscard]] - protocol_autogen::Optional - getDestinationTag() const + protocol_autogen::Optional + getBytecode() const { - if (hasDestinationTag()) + if (hasBytecode()) { - return this->tx_->at(sfDestinationTag); + return this->tx_->at(sfBytecode); } return std::nullopt; } /** - * @brief Check if sfDestinationTag is present. + * @brief Check if sfBytecode is present. * @return True if the field is present, false otherwise. */ [[nodiscard]] bool - hasDestinationTag() const + hasBytecode() const { - return this->tx_->isFieldPresent(sfDestinationTag); + return this->tx_->isFieldPresent(sfBytecode); + } + + /** + * @brief Get sfData (SoeOptional) + * @return The field value, or std::nullopt if not present. + */ + [[nodiscard]] + protocol_autogen::Optional + getData() const + { + if (hasData()) + { + return this->tx_->at(sfData); + } + return std::nullopt; + } + + /** + * @brief Check if sfData is present. + * @return True if the field is present, false otherwise. + */ + [[nodiscard]] + bool + hasData() const + { + return this->tx_->isFieldPresent(sfData); } }; @@ -232,6 +284,17 @@ public: return *this; } + /** + * @brief Set sfDestinationTag (SoeOptional) + * @return Reference to this builder for method chaining. + */ + EscrowCreateBuilder& + setDestinationTag(std::decay_t const& value) + { + object_[sfDestinationTag] = value; + return *this; + } + /** * @brief Set sfAmount (SoeRequired) * @note This field supports MPT (Multi-Purpose Token) amounts. @@ -278,13 +341,24 @@ public: } /** - * @brief Set sfDestinationTag (SoeOptional) + * @brief Set sfBytecode (SoeOptional) * @return Reference to this builder for method chaining. */ EscrowCreateBuilder& - setDestinationTag(std::decay_t const& value) + setBytecode(std::decay_t const& value) { - object_[sfDestinationTag] = value; + object_[sfBytecode] = value; + return *this; + } + + /** + * @brief Set sfData (SoeOptional) + * @return Reference to this builder for method chaining. + */ + EscrowCreateBuilder& + setData(std::decay_t const& value) + { + object_[sfData] = value; return *this; } diff --git a/include/xrpl/protocol_autogen/transactions/EscrowFinish.h b/include/xrpl/protocol_autogen/transactions/EscrowFinish.h index 1cbc60c738..9c24c7671b 100644 --- a/include/xrpl/protocol_autogen/transactions/EscrowFinish.h +++ b/include/xrpl/protocol_autogen/transactions/EscrowFinish.h @@ -146,6 +146,32 @@ public: { return this->tx_->isFieldPresent(sfCredentialIDs); } + + /** + * @brief Get sfGas (SoeOptional) + * @return The field value, or std::nullopt if not present. + */ + [[nodiscard]] + protocol_autogen::Optional + getGas() const + { + if (hasGas()) + { + return this->tx_->at(sfGas); + } + return std::nullopt; + } + + /** + * @brief Check if sfGas is present. + * @return True if the field is present, false otherwise. + */ + [[nodiscard]] + bool + hasGas() const + { + return this->tx_->isFieldPresent(sfGas); + } }; /** @@ -249,6 +275,17 @@ public: return *this; } + /** + * @brief Set sfGas (SoeOptional) + * @return Reference to this builder for method chaining. + */ + EscrowFinishBuilder& + setGas(std::decay_t const& value) + { + object_[sfGas] = value; + return *this; + } + /** * @brief Build and return the EscrowFinish wrapper. * @param publicKey The public key for signing. diff --git a/include/xrpl/protocol_autogen/transactions/SetFee.h b/include/xrpl/protocol_autogen/transactions/SetFee.h index 9513723e94..edcea7b734 100644 --- a/include/xrpl/protocol_autogen/transactions/SetFee.h +++ b/include/xrpl/protocol_autogen/transactions/SetFee.h @@ -254,6 +254,84 @@ public: { return this->tx_->isFieldPresent(sfReserveIncrementDrops); } + + /** + * @brief Get sfGasLimit (SoeOptional) + * @return The field value, or std::nullopt if not present. + */ + [[nodiscard]] + protocol_autogen::Optional + getGasLimit() const + { + if (hasGasLimit()) + { + return this->tx_->at(sfGasLimit); + } + return std::nullopt; + } + + /** + * @brief Check if sfGasLimit is present. + * @return True if the field is present, false otherwise. + */ + [[nodiscard]] + bool + hasGasLimit() const + { + return this->tx_->isFieldPresent(sfGasLimit); + } + + /** + * @brief Get sfBytecodeSizeLimit (SoeOptional) + * @return The field value, or std::nullopt if not present. + */ + [[nodiscard]] + protocol_autogen::Optional + getBytecodeSizeLimit() const + { + if (hasBytecodeSizeLimit()) + { + return this->tx_->at(sfBytecodeSizeLimit); + } + return std::nullopt; + } + + /** + * @brief Check if sfBytecodeSizeLimit is present. + * @return True if the field is present, false otherwise. + */ + [[nodiscard]] + bool + hasBytecodeSizeLimit() const + { + return this->tx_->isFieldPresent(sfBytecodeSizeLimit); + } + + /** + * @brief Get sfGasPrice (SoeOptional) + * @return The field value, or std::nullopt if not present. + */ + [[nodiscard]] + protocol_autogen::Optional + getGasPrice() const + { + if (hasGasPrice()) + { + return this->tx_->at(sfGasPrice); + } + return std::nullopt; + } + + /** + * @brief Check if sfGasPrice is present. + * @return True if the field is present, false otherwise. + */ + [[nodiscard]] + bool + hasGasPrice() const + { + return this->tx_->isFieldPresent(sfGasPrice); + } }; /** @@ -386,6 +464,39 @@ public: return *this; } + /** + * @brief Set sfGasLimit (SoeOptional) + * @return Reference to this builder for method chaining. + */ + SetFeeBuilder& + setGasLimit(std::decay_t const& value) + { + object_[sfGasLimit] = value; + return *this; + } + + /** + * @brief Set sfBytecodeSizeLimit (SoeOptional) + * @return Reference to this builder for method chaining. + */ + SetFeeBuilder& + setBytecodeSizeLimit(std::decay_t const& value) + { + object_[sfBytecodeSizeLimit] = value; + return *this; + } + + /** + * @brief Set sfGasPrice (SoeOptional) + * @return Reference to this builder for method chaining. + */ + SetFeeBuilder& + setGasPrice(std::decay_t const& value) + { + object_[sfGasPrice] = value; + return *this; + } + /** * @brief Build and return the SetFee wrapper. * @param publicKey The public key for signing. diff --git a/nix/check-tools/macos.txt b/nix/check-tools/macos.txt index 8edfeef311..d2dee651f1 100644 --- a/nix/check-tools/macos.txt +++ b/nix/check-tools/macos.txt @@ -131,6 +131,9 @@ Rust toolchain: ✅ rust-analyzer rust-analyzer 1.97.1 (8bab26f4 2026-07-14) /nix/store/j6apc5pmd0giy15da9p650r8zklslmvi-rust-analyzer-preview-1.97.1-aarch64-apple-darwin/bin/rust-analyzer + ✅ rust-nightly + rustc 1.99.0-nightly (87e5904f5 2026-07-20) + /nix/store/fqpjz4l0nsnji8b2pz57mnj0akbp6hcl-rust-nightly/bin/rust-nightly ✅ rustc rustc 1.97.1 (8bab26f4f 2026-07-14) /nix/store/bnfk1sl4s9angb0vj1cj9a5y5zvqinwy-rust-minimal-1.97.1/bin/rustc @@ -140,4 +143,4 @@ Rust toolchain: Skipping git-over-HTTPS check (CHECK_TOOLS_SKIP_CLONE is set). -✅ All 44 checked tools are present and runnable. +✅ All 45 checked tools are present and runnable. diff --git a/nix/check-tools/nix-ubuntu-amd64.txt b/nix/check-tools/nix-ubuntu-amd64.txt index 28b6c38014..ba5d5e65b1 100644 --- a/nix/check-tools/nix-ubuntu-amd64.txt +++ b/nix/check-tools/nix-ubuntu-amd64.txt @@ -131,6 +131,9 @@ Rust toolchain: ✅ rust-analyzer rust-analyzer 1.97.1 (8bab26f 2026-07-14) /nix/store/lr3m97p3hx1k22a7c44pb0wa7rbayhfi-rust-analyzer-preview-1.97.1-x86_64-unknown-linux-gnu/bin/rust-analyzer + ✅ rust-nightly + rustc 1.99.0-nightly (87e5904f5 2026-07-20) + /nix/store/j7kf7a5h4xypzp6x1skg4dsdx2k4fwb3-rust-nightly/bin/rust-nightly ✅ rustc rustc 1.97.1 (8bab26f4f 2026-07-14) /nix/store/40d3mzka7r1ps71l0yv2fs6616nbw85m-rust-minimal-1.97.1/bin/rustc @@ -168,4 +171,4 @@ Mold: Skipping git-over-HTTPS check (CHECK_TOOLS_SKIP_CLONE is set). -✅ All 52 checked tools are present and runnable. +✅ All 53 checked tools are present and runnable. diff --git a/nix/check-tools/nix-ubuntu-arm64.txt b/nix/check-tools/nix-ubuntu-arm64.txt index b3b5885a7f..2b45230327 100644 --- a/nix/check-tools/nix-ubuntu-arm64.txt +++ b/nix/check-tools/nix-ubuntu-arm64.txt @@ -131,6 +131,9 @@ Rust toolchain: ✅ rust-analyzer rust-analyzer 1.97.1 (8bab26f 2026-07-14) /nix/store/262830dlw2517lnagfx7i7agqgl4fmsd-rust-analyzer-preview-1.97.1-aarch64-unknown-linux-gnu/bin/rust-analyzer + ✅ rust-nightly + rustc 1.99.0-nightly (87e5904f5 2026-07-20) + /nix/store/c59pxk1yikdlf129qwyg4fplmxcrha0k-rust-nightly/bin/rust-nightly ✅ rustc rustc 1.97.1 (8bab26f4f 2026-07-14) /nix/store/a6p27cg6b8szfixfyvkssx6l0c345zw8-rust-minimal-1.97.1/bin/rustc @@ -168,4 +171,4 @@ Mold: Skipping git-over-HTTPS check (CHECK_TOOLS_SKIP_CLONE is set). -✅ All 52 checked tools are present and runnable. +✅ All 53 checked tools are present and runnable. diff --git a/src/libxrpl/protocol/STValidation.cpp b/src/libxrpl/protocol/STValidation.cpp index 1656aad3a2..9fdb6e4cee 100644 --- a/src/libxrpl/protocol/STValidation.cpp +++ b/src/libxrpl/protocol/STValidation.cpp @@ -59,6 +59,10 @@ STValidation::validationFormat() {sfBaseFeeDrops, SoeOptional}, {sfReserveBaseDrops, SoeOptional}, {sfReserveIncrementDrops, SoeOptional}, + // featureSmartEscrow + {sfGasLimit, SoeOptional}, + {sfBytecodeSizeLimit, SoeOptional}, + {sfGasPrice, SoeOptional}, }; // clang-format on diff --git a/src/libxrpl/protocol/TER.cpp b/src/libxrpl/protocol/TER.cpp index c2167d58ce..c6ebe98642 100644 --- a/src/libxrpl/protocol/TER.cpp +++ b/src/libxrpl/protocol/TER.cpp @@ -108,6 +108,8 @@ transResults() MAKE_ERROR(tecPRECISION_LOSS, "The amounts used by the transaction cannot interact."), MAKE_ERROR(tecBAD_PROOF, "Proof cannot be verified"), MAKE_ERROR(tecNO_SPONSOR_PERMISSION, "Sponsor has not authorized this transaction."), + MAKE_ERROR(tecOUT_OF_GAS, "The WASM code ran out of gas during execution."), + MAKE_ERROR(tecBYTECODE_REJECTED, "The custom WASM code that was run rejected your transaction."), MAKE_ERROR(tefALREADY, "The exact transaction was already in this ledger."), MAKE_ERROR(tefBAD_ADD_AUTH, "Not authorized to add account."), @@ -133,6 +135,8 @@ transResults() MAKE_ERROR(tefINVALID_LEDGER_FIX_TYPE, "The LedgerFixType field has an invalid value."), MAKE_ERROR(tefNO_DST_PARTIAL, "Partial payment to create account not allowed."), MAKE_ERROR(tefBAD_PATH_COUNT, "Malformed: Too many paths."), + MAKE_ERROR(tefNO_BYTECODE, "There is no WASM code to run, but a WASM-specific field was included."), + MAKE_ERROR(tefBYTECODE_NOT_INCLUDED, "WASM code requires a field that was not included."), MAKE_ERROR(telLOCAL_ERROR, "Local failure."), MAKE_ERROR(telBAD_DOMAIN, "Domain too long."), @@ -204,6 +208,8 @@ transResults() MAKE_ERROR(temBAD_TRANSFER_FEE, "Malformed: Transfer fee is outside valid range."), MAKE_ERROR(temINVALID_INNER_BATCH, "Malformed: Invalid inner batch transaction."), MAKE_ERROR(temBAD_CIPHERTEXT, "Malformed: Invalid ciphertext."), + MAKE_ERROR(temINVALID_BYTECODE, "Malformed: Provided byte code is invalid."), + MAKE_ERROR(temTEMP_DISABLED, "The transaction requires logic that is currently temporarily disabled."), MAKE_ERROR(terRETRY, "Retry transaction."), MAKE_ERROR(terFUNDS_SPENT, "DEPRECATED."), diff --git a/src/libxrpl/tx/applySteps.cpp b/src/libxrpl/tx/applySteps.cpp index 40b0ef57f6..fb237861d7 100644 --- a/src/libxrpl/tx/applySteps.cpp +++ b/src/libxrpl/tx/applySteps.cpp @@ -331,8 +331,10 @@ preflight( // outside of a dry run. if ((flags & TapProposal) != TapNone && (flags & TapDryRun) == TapNone) { + // LCOV_EXCL_START JLOG(j.fatal()) << "apply (preflight): TapProposal set without TapDryRun."; return {pfCtx, {tefEXCEPTION, TxConsequences{tx}}}; + // LCOV_EXCL_STOP } try @@ -360,8 +362,10 @@ preflight( // See the comment in the other preflight() overload above. if ((flags & TapProposal) != TapNone && (flags & TapDryRun) == TapNone) { + // LCOV_EXCL_START JLOG(j.fatal()) << "apply (preflight): TapProposal set without TapDryRun."; return {pfCtx, {tefEXCEPTION, TxConsequences{tx}}}; + // LCOV_EXCL_STOP } try @@ -384,16 +388,16 @@ invokeCheckPermission(ReadView const& view, STTx const& tx) return Transactor::invokeCheckPermission(view, tx); }); } + // LCOV_EXCL_START catch (UnknownTxnType const& e) { // Should never happen - // LCOV_EXCL_START JLOG(debugLog().fatal()) << "Unknown transaction type in invokeCheckPermission: " << e.txnType; UNREACHABLE("xrpl::invokeCheckPermission : unknown transaction type"); return temUNKNOWN; - // LCOV_EXCL_STOP } + // LCOV_EXCL_STOP } PreclaimResult diff --git a/src/libxrpl/tx/transactors/escrow/EscrowCreate.cpp b/src/libxrpl/tx/transactors/escrow/EscrowCreate.cpp index 0fe27fb3ba..589ad70230 100644 --- a/src/libxrpl/tx/transactors/escrow/EscrowCreate.cpp +++ b/src/libxrpl/tx/transactors/escrow/EscrowCreate.cpp @@ -88,8 +88,13 @@ EscrowCreate::checkExtraFeatures(PreflightContext const& ctx) // Only require featureMPTokensV1 when the escrow amount is an MPT and // fixCleanup3_2_0 is active; XRP/IOU escrows are unaffected by this gate. if (ctx.rules.enabled(fixCleanup3_2_0) && ctx.tx[sfAmount].holds()) - return ctx.rules.enabled(featureMPTokensV1); - return true; + { + if (!ctx.rules.enabled(featureMPTokensV1)) + return false; + } + + return (!ctx.tx.isFieldPresent(sfBytecode) && !ctx.tx.isFieldPresent(sfData)) || + ctx.rules.enabled(featureSmartEscrow); } template diff --git a/src/libxrpl/tx/transactors/escrow/EscrowFinish.cpp b/src/libxrpl/tx/transactors/escrow/EscrowFinish.cpp index 09219b0bf1..59bec8ca4c 100644 --- a/src/libxrpl/tx/transactors/escrow/EscrowFinish.cpp +++ b/src/libxrpl/tx/transactors/escrow/EscrowFinish.cpp @@ -65,7 +65,13 @@ checkCondition(Slice f, Slice c) bool EscrowFinish::checkExtraFeatures(PreflightContext const& ctx) { - return !ctx.tx.isFieldPresent(sfCredentialIDs) || ctx.rules.enabled(featureCredentials); + if (ctx.tx.isFieldPresent(sfCredentialIDs) && !ctx.rules.enabled(featureCredentials)) + return false; + + if (ctx.tx.isFieldPresent(sfGas) && !ctx.rules.enabled(featureSmartEscrow)) + return false; + + return true; } NotTEC diff --git a/src/libxrpl/tx/transactors/proposal/TransactionProposalCreate.cpp b/src/libxrpl/tx/transactors/proposal/TransactionProposalCreate.cpp index e9d04c41f5..4b028420f5 100644 --- a/src/libxrpl/tx/transactors/proposal/TransactionProposalCreate.cpp +++ b/src/libxrpl/tx/transactors/proposal/TransactionProposalCreate.cpp @@ -205,6 +205,10 @@ TransactionProposalCreate::preclaim(PreclaimContext const& ctx) SignerEntries::deserialize(*sleSigners, ctx.j, "ledger"); if (!accountSigners) { + // Only reachable if the on-ledger SignerList is corrupt + // (SignerListSet re-runs the same deserialize on write). + // Exercised by testCorruptSignerList via an OpenLedger + // overlay that produces the same failure modes. JLOG(ctx.j.fatal()) << "TransactionProposalCreate: unparseable SignerList: " << transToken(accountSigners.error()); return std::unexpected(tefBAD_LEDGER); @@ -215,6 +219,9 @@ TransactionProposalCreate::preclaim(PreclaimContext const& ctx) } catch (std::exception const& e) { + // Same as above: only reachable via ledger corruption that + // makes an STObject accessor throw. Exercised by + // testCorruptSignerList. JLOG(ctx.j.fatal()) << "TransactionProposalCreate: unparseable SignerList: " << e.what(); return std::unexpected(tefBAD_LEDGER); diff --git a/src/libxrpl/tx/transactors/system/Change.cpp b/src/libxrpl/tx/transactors/system/Change.cpp index f27855a5c8..0d50b80af4 100644 --- a/src/libxrpl/tx/transactors/system/Change.cpp +++ b/src/libxrpl/tx/transactors/system/Change.cpp @@ -123,6 +123,12 @@ Change::preclaim(PreclaimContext const& ctx) ctx.tx.isFieldPresent(sfReserveIncrementDrops)) return temDISABLED; } + // The ttFEE transaction format defines these fields as optional, + // but they are unconditionally forbidden until FeeVoteImpl is + // updated to populate them (SmartEscrow behavioral port). + if (ctx.tx.isFieldPresent(sfGasLimit) || ctx.tx.isFieldPresent(sfBytecodeSizeLimit) || + ctx.tx.isFieldPresent(sfGasPrice)) + return temDISABLED; return tesSUCCESS; case ttAMENDMENT: case ttUNL_MODIFY: diff --git a/src/test/app/TransactionProposalCreate_test.cpp b/src/test/app/TransactionProposalCreate_test.cpp index 9e13a4b90d..dad6ab943f 100644 --- a/src/test/app/TransactionProposalCreate_test.cpp +++ b/src/test/app/TransactionProposalCreate_test.cpp @@ -976,6 +976,72 @@ struct TransactionProposalCreate_test : public beast::unit_test::Suite } } + // Same failure mode as testCorruptSignerList, but reached through the + // delegate branch: preclaim looks up the delegate's own SignerList when + // the proposer is neither the target, on the target's SignerList, nor the + // delegate itself. If the delegate's SignerList is unparseable, preclaim + // must surface tefBAD_LEDGER — exercising the second isAuthorizedFor call + // that runs against the delegate rather than the target. + void + testCorruptDelegateSignerList(FeatureBitset features) + { + testcase("unparseable delegate SignerList is tefBAD_LEDGER"); + + using namespace jtx; + using namespace std::chrono_literals; + + Env env{*this, features}; + + Account const target{"target"}; + Account const delegateAcct{"delegateAcct"}; + Account const ds1{"ds1"}; // on delegateAcct's own SignerList, not target's + Account const bob{"bob"}; + env.fund(XRP(10000), target, delegateAcct, ds1, bob); + env.close(); + + // Grant delegate Payment permission for target; give delegate its own + // SignerList so that isAuthorizedFor(delegateAccount) actually reads + // and deserializes it. Do not give target a SignerList: proposer ds1 + // must fail isAuthorizedFor(target) before ever reaching the delegate + // branch. + env(delegate::set(target, delegateAcct, {"Payment"})); + env(signers(delegateAcct, 1, {{ds1, 1}})); + env.close(); + + // Ticket first: createTicket closes, which would drop a later + // open-ledger overlay and restore a well-formed SignerList. + std::uint32_t const ticketSeq = proposal::createTicket(env, target); + + // Corrupt delegate's SignerList in the open ledger overlay only. Do + // not close() afterward: a closed ledger would drop the overlay. + auto const delegateSignerListKeylet = keylet::signerList(delegateAcct.id()); + BEAST_EXPECT(env.app().getOpenLedger().modify([&](OpenView& view, beast::Journal) { + auto const sle = view.read(delegateSignerListKeylet); + if (!sle) + return false; + auto replacement = std::make_shared(*sle); + // Right inner name, but no sfAccount: deserialize calls + // getAccountID and throws (Field not found), which the catch + // maps to tefBAD_LEDGER. + STArray badEntries; + badEntries.pushBack(STObject{sfSignerEntry}); + replacement->setFieldArray(sfSignerEntries, badEntries); + view.rawReplace(replacement); + return true; + })); + BEAST_EXPECT(env.le(delegateSignerListKeylet)); + + json::Value tx = pay(target, bob, XRP(1)); + tx[sfDelegate.jsonName] = delegateAcct.human(); + env(proposal::create( + ds1, + proposal::unsignedPayload(env, tx, ticketSeq), + proposal::expiration(env, 100s)), + Ter(tefBAD_LEDGER), + proposal::verify::create()); + BEAST_EXPECT(!proposal::entry(env, target, ticketSeq)); + } + // The target account must be able to authorize a transaction through a // SignerList, so a pseudo-account (here an AMM's) cannot be a target even // though it exists on-ledger (On-Chain Cosigner spec §5.3.2.5). @@ -1566,6 +1632,7 @@ struct TransactionProposalCreate_test : public beast::unit_test::Suite testCorruptSignerList(all); testDelegatedProposedTx(all); testDelegatedGranularProposedTx(all); + testCorruptDelegateSignerList(all); testPseudoTarget(all); // Apply diff --git a/src/test/rpc/LedgerEntry_test.cpp b/src/test/rpc/LedgerEntry_test.cpp index 24dde05ce1..3abe83153b 100644 --- a/src/test/rpc/LedgerEntry_test.cpp +++ b/src/test/rpc/LedgerEntry_test.cpp @@ -15,6 +15,7 @@ #include #include #include +#include #include #include #include @@ -327,6 +328,7 @@ class LedgerEntry_test : public beast::unit_test::Suite FieldType const typeID, std::string const& expectedError, bool required = true, + std::optional typeNameOverride = std::nullopt, std::source_location const location = std::source_location::current()) { forAllApiVersions([&, this](unsigned apiVersion) { @@ -349,8 +351,8 @@ class LedgerEntry_test : public beast::unit_test::Suite correctRequest[fieldName] = fieldValue; json::Value const jrr = env.rpc( apiVersion, "json", "ledger_entry", to_string(correctRequest))[jss::result]; - auto const expectedErrMsg = - rpc::expectedFieldMessage(fieldName, getTypeName(typeID)); + auto const expectedErrMsg = rpc::expectedFieldMessage( + fieldName, typeNameOverride.value_or(getTypeName(typeID))); checkErrorValue(jrr, expectedError, expectedErrMsg, location); }; @@ -426,6 +428,7 @@ class LedgerEntry_test : public beast::unit_test::Suite FieldType::HashField, "malformedRequest", true, + std::nullopt, location); } @@ -441,6 +444,7 @@ class LedgerEntry_test : public beast::unit_test::Suite test::jtx::Env& env, json::StaticString const& parentField, std::vector const& subfields, + std::optional parentTypeNameOverride = std::nullopt, std::source_location const location = std::source_location::current()) { testMalformedField( @@ -450,6 +454,7 @@ class LedgerEntry_test : public beast::unit_test::Suite FieldType::HashOrObjectField, "malformedRequest", true, + parentTypeNameOverride, location); json::Value correctOutput; @@ -2027,6 +2032,87 @@ class LedgerEntry_test : public beast::unit_test::Suite } } + void + testTransactionProposal() + { + testcase("TransactionProposal"); + using namespace test::jtx; + using namespace std::literals::chrono_literals; + + Env env{*this}; + + Account const target{"target"}; + Account const bob{"bob"}; + env.fund(XRP(10000), target, bob); + env.close(); + + // A ticket for the proposal to be built against, and the proposal + // itself (an unsigned Payment payload). + std::uint32_t const ticketSeq = proposal::createTicket(env, target); + env(proposal::create( + target, + proposal::unsignedPayload(env, pay(target, bob, XRP(1)), ticketSeq), + proposal::expiration(env, 100s)), + proposal::verify::create()); + env.close(); + + std::string const ledgerHash{to_string(env.closed()->header().hash)}; + auto const proposalIndex = to_string(keylet::txProposal(target.id(), ticketSeq).key); + + { + // Request by target account and ticket sequence. + json::Value jvParams; + jvParams[jss::transaction_proposal][jss::account] = target.human(); + jvParams[jss::transaction_proposal][jss::ticket_seq] = ticketSeq; + jvParams[jss::ledger_hash] = ledgerHash; + auto const jrr = env.rpc("json", "ledger_entry", to_string(jvParams))[jss::result]; + BEAST_EXPECT(jrr[jss::node][sfLedgerEntryType.jsonName] == jss::TransactionProposal); + BEAST_EXPECT(proposalIndex == jrr[jss::node][jss::index].asString()); + } + { + // Request by object index (hex string form). + json::Value jvParams; + jvParams[jss::transaction_proposal] = proposalIndex; + jvParams[jss::ledger_hash] = ledgerHash; + auto const jrr = env.rpc("json", "ledger_entry", to_string(jvParams))[jss::result]; + BEAST_EXPECT(jrr[jss::node][sfLedgerEntryType.jsonName] == jss::TransactionProposal); + BEAST_EXPECT(proposalIndex == jrr[jss::node][jss::index].asString()); + } + { + // No proposal exists against this (account, ticket_seq) pair. + json::Value jvParams; + jvParams[jss::transaction_proposal][jss::account] = target.human(); + jvParams[jss::transaction_proposal][jss::ticket_seq] = ticketSeq + 1; + jvParams[jss::ledger_hash] = ledgerHash; + auto const jrr = env.rpc("json", "ledger_entry", to_string(jvParams))[jss::result]; + checkErrorValue(jrr, "entryNotFound", "Entry not found."); + } + { + // Lookup by an index of the wrong entry type. + json::Value jvParams; + jvParams[jss::transaction_proposal] = to_string(keylet::account(target).key); + jvParams[jss::ledger_hash] = ledgerHash; + auto const jrr = env.rpc("json", "ledger_entry", to_string(jvParams))[jss::result]; + checkErrorValue(jrr, "unexpectedLedgerType", "Unexpected ledger type."); + } + + { + // Malformed cases (missing / wrong-type subfields, and a + // non-object non-hex-string parent value). Once the parent has + // been shown not to be an object, parseTransactionProposal names + // "hex string" — not "hex string or object" — as the form still + // on the table. + runLedgerEntryTest( + env, + jss::transaction_proposal, + { + {.fieldName = jss::account, .malformedErrorMsg = "malformedAddress"}, + {.fieldName = jss::ticket_seq, .malformedErrorMsg = "malformedRequest"}, + }, + "hex string"); + } + } + void testDID() { @@ -2747,6 +2833,7 @@ public: testSignerList(); testSponsorship(); testTicket(); + testTransactionProposal(); testDID(); testInvalidOracleLedgerEntry(); testOracleLedgerEntry(); diff --git a/src/tests/libxrpl/protocol_autogen/ledger_entries/EscrowTests.cpp b/src/tests/libxrpl/protocol_autogen/ledger_entries/EscrowTests.cpp index 2dbb450e28..ad5f88fd2f 100644 --- a/src/tests/libxrpl/protocol_autogen/ledger_entries/EscrowTests.cpp +++ b/src/tests/libxrpl/protocol_autogen/ledger_entries/EscrowTests.cpp @@ -27,6 +27,8 @@ TEST(EscrowTests, BuilderSettersRoundTrip) auto const conditionValue = canonical_VL(); auto const cancelAfterValue = canonical_UINT32(); auto const finishAfterValue = canonical_UINT32(); + auto const bytecodeValue = canonical_VL(); + auto const dataValue = canonical_VL(); auto const sourceTagValue = canonical_UINT32(); auto const destinationTagValue = canonical_UINT32(); auto const ownerNodeValue = canonical_UINT64(); @@ -49,6 +51,8 @@ TEST(EscrowTests, BuilderSettersRoundTrip) builder.setCondition(conditionValue); builder.setCancelAfter(cancelAfterValue); builder.setFinishAfter(finishAfterValue); + builder.setBytecode(bytecodeValue); + builder.setData(dataValue); builder.setSourceTag(sourceTagValue); builder.setDestinationTag(destinationTagValue); builder.setDestinationNode(destinationNodeValue); @@ -132,6 +136,22 @@ TEST(EscrowTests, BuilderSettersRoundTrip) EXPECT_TRUE(entry.hasFinishAfter()); } + { + auto const& expected = bytecodeValue; + auto const actualOpt = entry.getBytecode(); + ASSERT_TRUE(actualOpt.has_value()); + expectEqualField(expected, *actualOpt, "sfBytecode"); + EXPECT_TRUE(entry.hasBytecode()); + } + + { + auto const& expected = dataValue; + auto const actualOpt = entry.getData(); + ASSERT_TRUE(actualOpt.has_value()); + expectEqualField(expected, *actualOpt, "sfData"); + EXPECT_TRUE(entry.hasData()); + } + { auto const& expected = sourceTagValue; auto const actualOpt = entry.getSourceTag(); @@ -192,6 +212,8 @@ TEST(EscrowTests, BuilderFromSleRoundTrip) auto const conditionValue = canonical_VL(); auto const cancelAfterValue = canonical_UINT32(); auto const finishAfterValue = canonical_UINT32(); + auto const bytecodeValue = canonical_VL(); + auto const dataValue = canonical_VL(); auto const sourceTagValue = canonical_UINT32(); auto const destinationTagValue = canonical_UINT32(); auto const ownerNodeValue = canonical_UINT64(); @@ -210,6 +232,8 @@ TEST(EscrowTests, BuilderFromSleRoundTrip) sle->at(sfCondition) = conditionValue; sle->at(sfCancelAfter) = cancelAfterValue; sle->at(sfFinishAfter) = finishAfterValue; + sle->at(sfBytecode) = bytecodeValue; + sle->at(sfData) = dataValue; sle->at(sfSourceTag) = sourceTagValue; sle->at(sfDestinationTag) = destinationTagValue; sle->at(sfOwnerNode) = ownerNodeValue; @@ -340,6 +364,32 @@ TEST(EscrowTests, BuilderFromSleRoundTrip) expectEqualField(expected, *fromBuilderOpt, "sfFinishAfter"); } + { + auto const& expected = bytecodeValue; + + auto const fromSleOpt = entryFromSle.getBytecode(); + auto const fromBuilderOpt = entryFromBuilder.getBytecode(); + + ASSERT_TRUE(fromSleOpt.has_value()); + ASSERT_TRUE(fromBuilderOpt.has_value()); + + expectEqualField(expected, *fromSleOpt, "sfBytecode"); + expectEqualField(expected, *fromBuilderOpt, "sfBytecode"); + } + + { + auto const& expected = dataValue; + + auto const fromSleOpt = entryFromSle.getData(); + auto const fromBuilderOpt = entryFromBuilder.getData(); + + ASSERT_TRUE(fromSleOpt.has_value()); + ASSERT_TRUE(fromBuilderOpt.has_value()); + + expectEqualField(expected, *fromSleOpt, "sfData"); + expectEqualField(expected, *fromBuilderOpt, "sfData"); + } + { auto const& expected = sourceTagValue; @@ -477,6 +527,10 @@ TEST(EscrowTests, OptionalFieldsReturnNullopt) EXPECT_FALSE(entry.getCancelAfter().has_value()); EXPECT_FALSE(entry.hasFinishAfter()); EXPECT_FALSE(entry.getFinishAfter().has_value()); + EXPECT_FALSE(entry.hasBytecode()); + EXPECT_FALSE(entry.getBytecode().has_value()); + EXPECT_FALSE(entry.hasData()); + EXPECT_FALSE(entry.getData().has_value()); EXPECT_FALSE(entry.hasSourceTag()); EXPECT_FALSE(entry.getSourceTag().has_value()); EXPECT_FALSE(entry.hasDestinationTag()); diff --git a/src/tests/libxrpl/protocol_autogen/ledger_entries/FeeSettingsTests.cpp b/src/tests/libxrpl/protocol_autogen/ledger_entries/FeeSettingsTests.cpp index 479d0c56c6..92bf25398c 100644 --- a/src/tests/libxrpl/protocol_autogen/ledger_entries/FeeSettingsTests.cpp +++ b/src/tests/libxrpl/protocol_autogen/ledger_entries/FeeSettingsTests.cpp @@ -27,6 +27,9 @@ TEST(FeeSettingsTests, BuilderSettersRoundTrip) auto const baseFeeDropsValue = canonical_AMOUNT(); auto const reserveBaseDropsValue = canonical_AMOUNT(); auto const reserveIncrementDropsValue = canonical_AMOUNT(); + auto const gasLimitValue = canonical_UINT32(); + auto const bytecodeSizeLimitValue = canonical_UINT32(); + auto const gasPriceValue = canonical_UINT32(); auto const previousTxnIDValue = canonical_UINT256(); auto const previousTxnLgrSeqValue = canonical_UINT32(); @@ -40,6 +43,9 @@ TEST(FeeSettingsTests, BuilderSettersRoundTrip) builder.setBaseFeeDrops(baseFeeDropsValue); builder.setReserveBaseDrops(reserveBaseDropsValue); builder.setReserveIncrementDrops(reserveIncrementDropsValue); + builder.setGasLimit(gasLimitValue); + builder.setBytecodeSizeLimit(bytecodeSizeLimitValue); + builder.setGasPrice(gasPriceValue); builder.setPreviousTxnID(previousTxnIDValue); builder.setPreviousTxnLgrSeq(previousTxnLgrSeqValue); @@ -108,6 +114,30 @@ TEST(FeeSettingsTests, BuilderSettersRoundTrip) EXPECT_TRUE(entry.hasReserveIncrementDrops()); } + { + auto const& expected = gasLimitValue; + auto const actualOpt = entry.getGasLimit(); + ASSERT_TRUE(actualOpt.has_value()); + expectEqualField(expected, *actualOpt, "sfGasLimit"); + EXPECT_TRUE(entry.hasGasLimit()); + } + + { + auto const& expected = bytecodeSizeLimitValue; + auto const actualOpt = entry.getBytecodeSizeLimit(); + ASSERT_TRUE(actualOpt.has_value()); + expectEqualField(expected, *actualOpt, "sfBytecodeSizeLimit"); + EXPECT_TRUE(entry.hasBytecodeSizeLimit()); + } + + { + auto const& expected = gasPriceValue; + auto const actualOpt = entry.getGasPrice(); + ASSERT_TRUE(actualOpt.has_value()); + expectEqualField(expected, *actualOpt, "sfGasPrice"); + EXPECT_TRUE(entry.hasGasPrice()); + } + { auto const& expected = previousTxnIDValue; auto const actualOpt = entry.getPreviousTxnID(); @@ -144,6 +174,9 @@ TEST(FeeSettingsTests, BuilderFromSleRoundTrip) auto const baseFeeDropsValue = canonical_AMOUNT(); auto const reserveBaseDropsValue = canonical_AMOUNT(); auto const reserveIncrementDropsValue = canonical_AMOUNT(); + auto const gasLimitValue = canonical_UINT32(); + auto const bytecodeSizeLimitValue = canonical_UINT32(); + auto const gasPriceValue = canonical_UINT32(); auto const previousTxnIDValue = canonical_UINT256(); auto const previousTxnLgrSeqValue = canonical_UINT32(); @@ -156,6 +189,9 @@ TEST(FeeSettingsTests, BuilderFromSleRoundTrip) sle->at(sfBaseFeeDrops) = baseFeeDropsValue; sle->at(sfReserveBaseDrops) = reserveBaseDropsValue; sle->at(sfReserveIncrementDrops) = reserveIncrementDropsValue; + sle->at(sfGasLimit) = gasLimitValue; + sle->at(sfBytecodeSizeLimit) = bytecodeSizeLimitValue; + sle->at(sfGasPrice) = gasPriceValue; sle->at(sfPreviousTxnID) = previousTxnIDValue; sle->at(sfPreviousTxnLgrSeq) = previousTxnLgrSeqValue; @@ -259,6 +295,45 @@ TEST(FeeSettingsTests, BuilderFromSleRoundTrip) expectEqualField(expected, *fromBuilderOpt, "sfReserveIncrementDrops"); } + { + auto const& expected = gasLimitValue; + + auto const fromSleOpt = entryFromSle.getGasLimit(); + auto const fromBuilderOpt = entryFromBuilder.getGasLimit(); + + ASSERT_TRUE(fromSleOpt.has_value()); + ASSERT_TRUE(fromBuilderOpt.has_value()); + + expectEqualField(expected, *fromSleOpt, "sfGasLimit"); + expectEqualField(expected, *fromBuilderOpt, "sfGasLimit"); + } + + { + auto const& expected = bytecodeSizeLimitValue; + + auto const fromSleOpt = entryFromSle.getBytecodeSizeLimit(); + auto const fromBuilderOpt = entryFromBuilder.getBytecodeSizeLimit(); + + ASSERT_TRUE(fromSleOpt.has_value()); + ASSERT_TRUE(fromBuilderOpt.has_value()); + + expectEqualField(expected, *fromSleOpt, "sfBytecodeSizeLimit"); + expectEqualField(expected, *fromBuilderOpt, "sfBytecodeSizeLimit"); + } + + { + auto const& expected = gasPriceValue; + + auto const fromSleOpt = entryFromSle.getGasPrice(); + auto const fromBuilderOpt = entryFromBuilder.getGasPrice(); + + ASSERT_TRUE(fromSleOpt.has_value()); + ASSERT_TRUE(fromBuilderOpt.has_value()); + + expectEqualField(expected, *fromSleOpt, "sfGasPrice"); + expectEqualField(expected, *fromBuilderOpt, "sfGasPrice"); + } + { auto const& expected = previousTxnIDValue; @@ -351,6 +426,12 @@ TEST(FeeSettingsTests, OptionalFieldsReturnNullopt) EXPECT_FALSE(entry.getReserveBaseDrops().has_value()); EXPECT_FALSE(entry.hasReserveIncrementDrops()); EXPECT_FALSE(entry.getReserveIncrementDrops().has_value()); + EXPECT_FALSE(entry.hasGasLimit()); + EXPECT_FALSE(entry.getGasLimit().has_value()); + EXPECT_FALSE(entry.hasBytecodeSizeLimit()); + EXPECT_FALSE(entry.getBytecodeSizeLimit().has_value()); + EXPECT_FALSE(entry.hasGasPrice()); + EXPECT_FALSE(entry.getGasPrice().has_value()); EXPECT_FALSE(entry.hasPreviousTxnID()); EXPECT_FALSE(entry.getPreviousTxnID().has_value()); EXPECT_FALSE(entry.hasPreviousTxnLgrSeq()); diff --git a/src/tests/libxrpl/protocol_autogen/transactions/EscrowCreateTests.cpp b/src/tests/libxrpl/protocol_autogen/transactions/EscrowCreateTests.cpp index 1d62fadb20..44e3e200d8 100644 --- a/src/tests/libxrpl/protocol_autogen/transactions/EscrowCreateTests.cpp +++ b/src/tests/libxrpl/protocol_autogen/transactions/EscrowCreateTests.cpp @@ -30,11 +30,13 @@ TEST(TransactionsEscrowCreateTests, BuilderSettersRoundTrip) // Transaction-specific field values auto const destinationValue = canonical_ACCOUNT(); + auto const destinationTagValue = canonical_UINT32(); auto const amountValue = canonical_AMOUNT(); auto const conditionValue = canonical_VL(); auto const cancelAfterValue = canonical_UINT32(); auto const finishAfterValue = canonical_UINT32(); - auto const destinationTagValue = canonical_UINT32(); + auto const bytecodeValue = canonical_VL(); + auto const dataValue = canonical_VL(); EscrowCreateBuilder builder{ accountValue, @@ -45,10 +47,12 @@ TEST(TransactionsEscrowCreateTests, BuilderSettersRoundTrip) }; // Set optional fields + builder.setDestinationTag(destinationTagValue); builder.setCondition(conditionValue); builder.setCancelAfter(cancelAfterValue); builder.setFinishAfter(finishAfterValue); - builder.setDestinationTag(destinationTagValue); + builder.setBytecode(bytecodeValue); + builder.setData(dataValue); auto tx = builder.build(publicKey, secretKey); @@ -78,6 +82,14 @@ TEST(TransactionsEscrowCreateTests, BuilderSettersRoundTrip) } // Verify optional fields + { + auto const& expected = destinationTagValue; + auto const actualOpt = tx.getDestinationTag(); + ASSERT_TRUE(actualOpt.has_value()) << "Optional field sfDestinationTag should be present"; + expectEqualField(expected, *actualOpt, "sfDestinationTag"); + EXPECT_TRUE(tx.hasDestinationTag()); + } + { auto const& expected = conditionValue; auto const actualOpt = tx.getCondition(); @@ -103,11 +115,19 @@ TEST(TransactionsEscrowCreateTests, BuilderSettersRoundTrip) } { - auto const& expected = destinationTagValue; - auto const actualOpt = tx.getDestinationTag(); - ASSERT_TRUE(actualOpt.has_value()) << "Optional field sfDestinationTag should be present"; - expectEqualField(expected, *actualOpt, "sfDestinationTag"); - EXPECT_TRUE(tx.hasDestinationTag()); + auto const& expected = bytecodeValue; + auto const actualOpt = tx.getBytecode(); + ASSERT_TRUE(actualOpt.has_value()) << "Optional field sfBytecode should be present"; + expectEqualField(expected, *actualOpt, "sfBytecode"); + EXPECT_TRUE(tx.hasBytecode()); + } + + { + auto const& expected = dataValue; + auto const actualOpt = tx.getData(); + ASSERT_TRUE(actualOpt.has_value()) << "Optional field sfData should be present"; + expectEqualField(expected, *actualOpt, "sfData"); + EXPECT_TRUE(tx.hasData()); } } @@ -127,11 +147,13 @@ TEST(TransactionsEscrowCreateTests, BuilderFromStTxRoundTrip) // Transaction-specific field values auto const destinationValue = canonical_ACCOUNT(); + auto const destinationTagValue = canonical_UINT32(); auto const amountValue = canonical_AMOUNT(); auto const conditionValue = canonical_VL(); auto const cancelAfterValue = canonical_UINT32(); auto const finishAfterValue = canonical_UINT32(); - auto const destinationTagValue = canonical_UINT32(); + auto const bytecodeValue = canonical_VL(); + auto const dataValue = canonical_VL(); // Build an initial transaction EscrowCreateBuilder initialBuilder{ @@ -142,10 +164,12 @@ TEST(TransactionsEscrowCreateTests, BuilderFromStTxRoundTrip) feeValue }; + initialBuilder.setDestinationTag(destinationTagValue); initialBuilder.setCondition(conditionValue); initialBuilder.setCancelAfter(cancelAfterValue); initialBuilder.setFinishAfter(finishAfterValue); - initialBuilder.setDestinationTag(destinationTagValue); + initialBuilder.setBytecode(bytecodeValue); + initialBuilder.setData(dataValue); auto initialTx = initialBuilder.build(publicKey, secretKey); @@ -176,6 +200,13 @@ TEST(TransactionsEscrowCreateTests, BuilderFromStTxRoundTrip) } // Verify optional fields + { + auto const& expected = destinationTagValue; + auto const actualOpt = rebuiltTx.getDestinationTag(); + ASSERT_TRUE(actualOpt.has_value()) << "Optional field sfDestinationTag should be present"; + expectEqualField(expected, *actualOpt, "sfDestinationTag"); + } + { auto const& expected = conditionValue; auto const actualOpt = rebuiltTx.getCondition(); @@ -198,10 +229,17 @@ TEST(TransactionsEscrowCreateTests, BuilderFromStTxRoundTrip) } { - auto const& expected = destinationTagValue; - auto const actualOpt = rebuiltTx.getDestinationTag(); - ASSERT_TRUE(actualOpt.has_value()) << "Optional field sfDestinationTag should be present"; - expectEqualField(expected, *actualOpt, "sfDestinationTag"); + auto const& expected = bytecodeValue; + auto const actualOpt = rebuiltTx.getBytecode(); + ASSERT_TRUE(actualOpt.has_value()) << "Optional field sfBytecode should be present"; + expectEqualField(expected, *actualOpt, "sfBytecode"); + } + + { + auto const& expected = dataValue; + auto const actualOpt = rebuiltTx.getData(); + ASSERT_TRUE(actualOpt.has_value()) << "Optional field sfData should be present"; + expectEqualField(expected, *actualOpt, "sfData"); } } @@ -263,14 +301,18 @@ TEST(TransactionsEscrowCreateTests, OptionalFieldsReturnNullopt) auto tx = builder.build(publicKey, secretKey); // Verify optional fields are not present + EXPECT_FALSE(tx.hasDestinationTag()); + EXPECT_FALSE(tx.getDestinationTag().has_value()); EXPECT_FALSE(tx.hasCondition()); EXPECT_FALSE(tx.getCondition().has_value()); EXPECT_FALSE(tx.hasCancelAfter()); EXPECT_FALSE(tx.getCancelAfter().has_value()); EXPECT_FALSE(tx.hasFinishAfter()); EXPECT_FALSE(tx.getFinishAfter().has_value()); - EXPECT_FALSE(tx.hasDestinationTag()); - EXPECT_FALSE(tx.getDestinationTag().has_value()); + EXPECT_FALSE(tx.hasBytecode()); + EXPECT_FALSE(tx.getBytecode().has_value()); + EXPECT_FALSE(tx.hasData()); + EXPECT_FALSE(tx.getData().has_value()); } } diff --git a/src/tests/libxrpl/protocol_autogen/transactions/EscrowFinishTests.cpp b/src/tests/libxrpl/protocol_autogen/transactions/EscrowFinishTests.cpp index f76bfffb59..e062e24813 100644 --- a/src/tests/libxrpl/protocol_autogen/transactions/EscrowFinishTests.cpp +++ b/src/tests/libxrpl/protocol_autogen/transactions/EscrowFinishTests.cpp @@ -34,6 +34,7 @@ TEST(TransactionsEscrowFinishTests, BuilderSettersRoundTrip) auto const fulfillmentValue = canonical_VL(); auto const conditionValue = canonical_VL(); auto const credentialIDsValue = canonical_VECTOR256(); + auto const gasValue = canonical_UINT32(); EscrowFinishBuilder builder{ accountValue, @@ -47,6 +48,7 @@ TEST(TransactionsEscrowFinishTests, BuilderSettersRoundTrip) builder.setFulfillment(fulfillmentValue); builder.setCondition(conditionValue); builder.setCredentialIDs(credentialIDsValue); + builder.setGas(gasValue); auto tx = builder.build(publicKey, secretKey); @@ -100,6 +102,14 @@ TEST(TransactionsEscrowFinishTests, BuilderSettersRoundTrip) EXPECT_TRUE(tx.hasCredentialIDs()); } + { + auto const& expected = gasValue; + auto const actualOpt = tx.getGas(); + ASSERT_TRUE(actualOpt.has_value()) << "Optional field sfGas should be present"; + expectEqualField(expected, *actualOpt, "sfGas"); + EXPECT_TRUE(tx.hasGas()); + } + } // 2 & 4) Start from an STTx, construct a builder from it, build a new wrapper, @@ -121,6 +131,7 @@ TEST(TransactionsEscrowFinishTests, BuilderFromStTxRoundTrip) auto const fulfillmentValue = canonical_VL(); auto const conditionValue = canonical_VL(); auto const credentialIDsValue = canonical_VECTOR256(); + auto const gasValue = canonical_UINT32(); // Build an initial transaction EscrowFinishBuilder initialBuilder{ @@ -134,6 +145,7 @@ TEST(TransactionsEscrowFinishTests, BuilderFromStTxRoundTrip) initialBuilder.setFulfillment(fulfillmentValue); initialBuilder.setCondition(conditionValue); initialBuilder.setCredentialIDs(credentialIDsValue); + initialBuilder.setGas(gasValue); auto initialTx = initialBuilder.build(publicKey, secretKey); @@ -185,6 +197,13 @@ TEST(TransactionsEscrowFinishTests, BuilderFromStTxRoundTrip) expectEqualField(expected, *actualOpt, "sfCredentialIDs"); } + { + auto const& expected = gasValue; + auto const actualOpt = rebuiltTx.getGas(); + ASSERT_TRUE(actualOpt.has_value()) << "Optional field sfGas should be present"; + expectEqualField(expected, *actualOpt, "sfGas"); + } + } // 3) Verify wrapper throws when constructed from wrong transaction type. @@ -250,6 +269,8 @@ TEST(TransactionsEscrowFinishTests, OptionalFieldsReturnNullopt) EXPECT_FALSE(tx.getCondition().has_value()); EXPECT_FALSE(tx.hasCredentialIDs()); EXPECT_FALSE(tx.getCredentialIDs().has_value()); + EXPECT_FALSE(tx.hasGas()); + EXPECT_FALSE(tx.getGas().has_value()); } } diff --git a/src/tests/libxrpl/protocol_autogen/transactions/SetFeeTests.cpp b/src/tests/libxrpl/protocol_autogen/transactions/SetFeeTests.cpp index 9605daee9f..d7cfb469ba 100644 --- a/src/tests/libxrpl/protocol_autogen/transactions/SetFeeTests.cpp +++ b/src/tests/libxrpl/protocol_autogen/transactions/SetFeeTests.cpp @@ -37,6 +37,9 @@ TEST(TransactionsSetFeeTests, BuilderSettersRoundTrip) auto const baseFeeDropsValue = canonical_AMOUNT(); auto const reserveBaseDropsValue = canonical_AMOUNT(); auto const reserveIncrementDropsValue = canonical_AMOUNT(); + auto const gasLimitValue = canonical_UINT32(); + auto const bytecodeSizeLimitValue = canonical_UINT32(); + auto const gasPriceValue = canonical_UINT32(); SetFeeBuilder builder{ accountValue, @@ -53,6 +56,9 @@ TEST(TransactionsSetFeeTests, BuilderSettersRoundTrip) builder.setBaseFeeDrops(baseFeeDropsValue); builder.setReserveBaseDrops(reserveBaseDropsValue); builder.setReserveIncrementDrops(reserveIncrementDropsValue); + builder.setGasLimit(gasLimitValue); + builder.setBytecodeSizeLimit(bytecodeSizeLimitValue); + builder.setGasPrice(gasPriceValue); auto tx = builder.build(publicKey, secretKey); @@ -134,6 +140,30 @@ TEST(TransactionsSetFeeTests, BuilderSettersRoundTrip) EXPECT_TRUE(tx.hasReserveIncrementDrops()); } + { + auto const& expected = gasLimitValue; + auto const actualOpt = tx.getGasLimit(); + ASSERT_TRUE(actualOpt.has_value()) << "Optional field sfGasLimit should be present"; + expectEqualField(expected, *actualOpt, "sfGasLimit"); + EXPECT_TRUE(tx.hasGasLimit()); + } + + { + auto const& expected = bytecodeSizeLimitValue; + auto const actualOpt = tx.getBytecodeSizeLimit(); + ASSERT_TRUE(actualOpt.has_value()) << "Optional field sfBytecodeSizeLimit should be present"; + expectEqualField(expected, *actualOpt, "sfBytecodeSizeLimit"); + EXPECT_TRUE(tx.hasBytecodeSizeLimit()); + } + + { + auto const& expected = gasPriceValue; + auto const actualOpt = tx.getGasPrice(); + ASSERT_TRUE(actualOpt.has_value()) << "Optional field sfGasPrice should be present"; + expectEqualField(expected, *actualOpt, "sfGasPrice"); + EXPECT_TRUE(tx.hasGasPrice()); + } + } // 2 & 4) Start from an STTx, construct a builder from it, build a new wrapper, @@ -158,6 +188,9 @@ TEST(TransactionsSetFeeTests, BuilderFromStTxRoundTrip) auto const baseFeeDropsValue = canonical_AMOUNT(); auto const reserveBaseDropsValue = canonical_AMOUNT(); auto const reserveIncrementDropsValue = canonical_AMOUNT(); + auto const gasLimitValue = canonical_UINT32(); + auto const bytecodeSizeLimitValue = canonical_UINT32(); + auto const gasPriceValue = canonical_UINT32(); // Build an initial transaction SetFeeBuilder initialBuilder{ @@ -174,6 +207,9 @@ TEST(TransactionsSetFeeTests, BuilderFromStTxRoundTrip) initialBuilder.setBaseFeeDrops(baseFeeDropsValue); initialBuilder.setReserveBaseDrops(reserveBaseDropsValue); initialBuilder.setReserveIncrementDrops(reserveIncrementDropsValue); + initialBuilder.setGasLimit(gasLimitValue); + initialBuilder.setBytecodeSizeLimit(bytecodeSizeLimitValue); + initialBuilder.setGasPrice(gasPriceValue); auto initialTx = initialBuilder.build(publicKey, secretKey); @@ -248,6 +284,27 @@ TEST(TransactionsSetFeeTests, BuilderFromStTxRoundTrip) expectEqualField(expected, *actualOpt, "sfReserveIncrementDrops"); } + { + auto const& expected = gasLimitValue; + auto const actualOpt = rebuiltTx.getGasLimit(); + ASSERT_TRUE(actualOpt.has_value()) << "Optional field sfGasLimit should be present"; + expectEqualField(expected, *actualOpt, "sfGasLimit"); + } + + { + auto const& expected = bytecodeSizeLimitValue; + auto const actualOpt = rebuiltTx.getBytecodeSizeLimit(); + ASSERT_TRUE(actualOpt.has_value()) << "Optional field sfBytecodeSizeLimit should be present"; + expectEqualField(expected, *actualOpt, "sfBytecodeSizeLimit"); + } + + { + auto const& expected = gasPriceValue; + auto const actualOpt = rebuiltTx.getGasPrice(); + ASSERT_TRUE(actualOpt.has_value()) << "Optional field sfGasPrice should be present"; + expectEqualField(expected, *actualOpt, "sfGasPrice"); + } + } // 3) Verify wrapper throws when constructed from wrong transaction type. @@ -319,6 +376,12 @@ TEST(TransactionsSetFeeTests, OptionalFieldsReturnNullopt) EXPECT_FALSE(tx.getReserveBaseDrops().has_value()); EXPECT_FALSE(tx.hasReserveIncrementDrops()); EXPECT_FALSE(tx.getReserveIncrementDrops().has_value()); + EXPECT_FALSE(tx.hasGasLimit()); + EXPECT_FALSE(tx.getGasLimit().has_value()); + EXPECT_FALSE(tx.hasBytecodeSizeLimit()); + EXPECT_FALSE(tx.getBytecodeSizeLimit().has_value()); + EXPECT_FALSE(tx.hasGasPrice()); + EXPECT_FALSE(tx.getGasPrice().has_value()); } } diff --git a/src/tests/libxrpl/tx/ProposalHelpers.cpp b/src/tests/libxrpl/tx/ProposalHelpers.cpp new file mode 100644 index 0000000000..5dbb1461ad --- /dev/null +++ b/src/tests/libxrpl/tx/ProposalHelpers.cpp @@ -0,0 +1,116 @@ +#include + +#include +#include +#include +#include +#include + +#include + +#include +#include + +namespace xrpl::test { + +namespace { + +// A bare STObject carrying only sfTransactionType. isValidProposal accepts an +// STObject (not an STTx) precisely so that this file's defense-in-depth +// checks can be tested without the surrounding STTx format validation. +inline STObject +txOfType(std::uint16_t txType) +{ + STObject tx(sfGeneric); + tx.setFieldU16(sfTransactionType, txType); + return tx; +} + +// A bare Batch STObject wrapping the given inner transaction as its single +// sfRawTransactions entry. +inline STObject +batchWrapping(STObject inner) +{ + STArray rawTxns(sfRawTransactions); + rawTxns.push_back(std::move(inner)); + + STObject batch(sfGeneric); + batch.setFieldU16(sfTransactionType, ttBATCH); + batch.setFieldArray(sfRawTransactions, rawTxns); + return batch; +} + +} // namespace + +// The happy path — an ordinary Payment is independently submittable. +TEST(ProposalHelpers, PlainPaymentIsValid) +{ + EXPECT_TRUE(proposal::isValidProposal(txOfType(ttPAYMENT))); +} + +// A nested TransactionProposalCreate. In practice STTx construction rejects +// this earlier (the payload lacks TransactionProposalCreate's own template +// fields), but the defense here re-checks that guard so the two cannot +// drift apart. +TEST(ProposalHelpers, NestedProposalIsRejected) +{ + EXPECT_FALSE(proposal::isValidProposal(txOfType(ttTRANSACTION_PROPOSAL_CREATE))); +} + +// Any pseudo-transaction — see STTx::isPseudoTx. Also normally caught earlier +// by STTx construction / preflight0. +TEST(ProposalHelpers, PseudoTxIsRejected) +{ + EXPECT_FALSE(proposal::isValidProposal(txOfType(ttAMENDMENT))); + EXPECT_FALSE(proposal::isValidProposal(txOfType(ttFEE))); + EXPECT_FALSE(proposal::isValidProposal(txOfType(ttUNL_MODIFY))); +} + +// tfInnerBatchTxn marks a transaction as an inner leg of an enclosing Batch, +// so it must never stand on its own as a proposed transaction. preflight0 +// rejects the standalone case with temINVALID_INNER_BATCH before we get +// here; the guard is re-checked so the two cannot drift apart. +TEST(ProposalHelpers, InnerBatchFlagIsRejected) +{ + STObject tx = txOfType(ttPAYMENT); + tx.setFieldU32(sfFlags, tfInnerBatchTxn); + EXPECT_FALSE(proposal::isValidProposal(tx)); +} + +// A Flags value that is present but does not include tfInnerBatchTxn must +// not be rejected — the check is bit-specific, not "any flag present". +TEST(ProposalHelpers, OtherFlagsAreAccepted) +{ + STObject tx = txOfType(ttPAYMENT); + tx.setFieldU32(sfFlags, tfFullyCanonicalSig); + EXPECT_TRUE(proposal::isValidProposal(tx)); +} + +// A Batch wrapping a plain inner is fine — the loop is only there to catch +// specifically forbidden inner types. +TEST(ProposalHelpers, BatchWithPlainInnerIsValid) +{ + EXPECT_TRUE(proposal::isValidProposal(batchWrapping(txOfType(ttPAYMENT)))); +} + +// A Batch whose inner is itself a proposal must be rejected. +TEST(ProposalHelpers, BatchWithNestedProposalInnerIsRejected) +{ + EXPECT_FALSE(proposal::isValidProposal(batchWrapping(txOfType(ttTRANSACTION_PROPOSAL_CREATE)))); +} + +// A Batch whose inner is a pseudo-transaction must be rejected. +TEST(ProposalHelpers, BatchWithPseudoInnerIsRejected) +{ + EXPECT_FALSE(proposal::isValidProposal(batchWrapping(txOfType(ttAMENDMENT)))); +} + +// A Batch with no sfRawTransactions field skips the inner-loop entirely. +// Not something the transactor would ever emit, but the branch exists in the +// helper (the field is optional at the STObject level) and should hold. +TEST(ProposalHelpers, BatchWithoutRawTransactionsIsValid) +{ + EXPECT_TRUE(proposal::isValidProposal(txOfType(ttBATCH))); +} + +} // namespace xrpl::test diff --git a/src/xrpld/rpc/handlers/ledger/LedgerEntry.cpp b/src/xrpld/rpc/handlers/ledger/LedgerEntry.cpp index a92b828556..f56a8825bb 100644 --- a/src/xrpld/rpc/handlers/ledger/LedgerEntry.cpp +++ b/src/xrpld/rpc/handlers/ledger/LedgerEntry.cpp @@ -757,6 +757,11 @@ parseTransactionProposal( json::StaticString const fieldName, [[maybe_unused]] unsigned const apiVersion) { + // In the non-object branch the caller must supply the proposal ID as a + // hex string (the object form is the {account, ticket_seq} pair handled + // below). Passing "hex string" here — rather than the more general + // "hex string or object" default — makes the error message name the + // exact form still on the table once an object has been ruled out. if (!params.isObject()) return parseObjectID(params, fieldName, "hex string");