Merge remote-tracking branch 'mywork/ximinez/lending-tx-fix' into ximinez/lending-XLS-66

* mywork/ximinez/lending-tx-fix:
  fix: Transaction sig checking functions do not get a full context
  ci: Upload artifacts during build and test in a separate job (5817)
This commit is contained in:
Ed Hennis
2025-09-30 16:28:10 -04:00
12 changed files with 386 additions and 266 deletions

View File

@@ -1,96 +0,0 @@
# This action build and tests the binary. The Conan dependencies must have
# already been installed (see the build-deps action).
name: Build and Test
description: "Build and test the binary."
# Note that actions do not support 'type' and all inputs are strings, see
# https://docs.github.com/en/actions/reference/workflows-and-actions/metadata-syntax#inputs.
inputs:
build_dir:
description: "The directory where to build."
required: true
build_only:
description: 'Whether to only build or to build and test the code ("true", "false").'
required: false
default: "false"
build_type:
description: 'The build type to use ("Debug", "Release").'
required: true
cmake_args:
description: "Additional arguments to pass to CMake."
required: false
default: ""
cmake_target:
description: "The CMake target to build."
required: true
codecov_token:
description: "The Codecov token to use for uploading coverage reports."
required: false
default: ""
os:
description: 'The operating system to use for the build ("linux", "macos", "windows").'
required: true
runs:
using: composite
steps:
- name: Configure CMake
shell: bash
working-directory: ${{ inputs.build_dir }}
run: |
echo 'Configuring CMake.'
cmake \
-G '${{ inputs.os == 'windows' && 'Visual Studio 17 2022' || 'Ninja' }}' \
-DCMAKE_TOOLCHAIN_FILE:FILEPATH=build/generators/conan_toolchain.cmake \
-DCMAKE_BUILD_TYPE=${{ inputs.build_type }} \
${{ inputs.cmake_args }} \
..
- name: Build the binary
shell: bash
working-directory: ${{ inputs.build_dir }}
run: |
echo 'Building binary.'
cmake \
--build . \
--config ${{ inputs.build_type }} \
--parallel $(nproc) \
--target ${{ inputs.cmake_target }}
- name: Check linking
if: ${{ inputs.os == 'linux' }}
shell: bash
working-directory: ${{ inputs.build_dir }}
run: |
echo 'Checking linking.'
ldd ./rippled
if [ "$(ldd ./rippled | grep -E '(libstdc\+\+|libgcc)' | wc -l)" -eq 0 ]; then
echo 'The binary is statically linked.'
else
echo 'The binary is dynamically linked.'
exit 1
fi
- name: Verify voidstar
if: ${{ contains(inputs.cmake_args, '-Dvoidstar=ON') }}
shell: bash
working-directory: ${{ inputs.build_dir }}
run: |
echo 'Verifying presence of instrumentation.'
./rippled --version | grep libvoidstar
- name: Test the binary
if: ${{ inputs.build_only == 'false' }}
shell: bash
working-directory: ${{ inputs.build_dir }}/${{ inputs.os == 'windows' && inputs.build_type || '' }}
run: |
echo 'Testing binary.'
./rippled --unittest --unittest-jobs $(nproc)
ctest -j $(nproc) --output-on-failure
- name: Upload coverage report
if: ${{ inputs.cmake_target == 'coverage' }}
uses: codecov/codecov-action@18283e04ce6e62d37312384ff67231eb8fd56d24 # v5.4.3
with:
disable_search: true
disable_telem: true
fail_ci_if_error: true
files: ${{ inputs.build_dir }}/coverage.xml
plugins: noop
token: ${{ inputs.codecov_token }}
verbose: true

43
.github/actions/print-env/action.yml vendored Normal file
View File

@@ -0,0 +1,43 @@
name: Print build environment
description: "Print environment and some tooling versions"
runs:
using: composite
steps:
- name: Check configuration (Windows)
if: ${{ runner.os == 'Windows' }}
shell: bash
run: |
echo 'Checking environment variables.'
set
echo 'Checking CMake version.'
cmake --version
echo 'Checking Conan version.'
conan --version
- name: Check configuration (Linux and macOS)
if: ${{ runner.os == 'Linux' || runner.os == 'macOS' }}
shell: bash
run: |
echo 'Checking path.'
echo ${PATH} | tr ':' '\n'
echo 'Checking environment variables.'
env | sort
echo 'Checking CMake version.'
cmake --version
echo 'Checking compiler version.'
${{ runner.os == 'Linux' && '${CC}' || 'clang' }} --version
echo 'Checking Conan version.'
conan --version
echo 'Checking Ninja version.'
ninja --version
echo 'Checking nproc version.'
nproc --version

View File

@@ -162,7 +162,7 @@ def generate_strategy_matrix(all: bool, config: Config) -> list:
'config_name': config_name, 'config_name': config_name,
'cmake_args': cmake_args, 'cmake_args': cmake_args,
'cmake_target': cmake_target, 'cmake_target': cmake_target,
'build_only': 'true' if build_only else 'false', 'build_only': build_only,
'build_type': build_type, 'build_type': build_type,
'os': os, 'os': os,
'architecture': architecture, 'architecture': architecture,

View File

@@ -59,8 +59,11 @@ jobs:
.github/actions/build-test/** .github/actions/build-test/**
.github/actions/setup-conan/** .github/actions/setup-conan/**
.github/scripts/strategy-matrix/** .github/scripts/strategy-matrix/**
.github/workflows/reusable-build.yml
.github/workflows/reusable-build-test-config.yml
.github/workflows/reusable-build-test.yml .github/workflows/reusable-build-test.yml
.github/workflows/reusable-strategy-matrix.yml .github/workflows/reusable-strategy-matrix.yml
.github/workflows/reusable-test.yml
.codecov.yml .codecov.yml
cmake/** cmake/**
conan/** conan/**
@@ -105,7 +108,7 @@ jobs:
with: with:
os: ${{ matrix.os }} os: ${{ matrix.os }}
secrets: secrets:
codecov_token: ${{ secrets.CODECOV_TOKEN }} CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }}
notify-clio: notify-clio:
needs: needs:

View File

@@ -23,8 +23,11 @@ on:
- ".github/actions/build-test/**" - ".github/actions/build-test/**"
- ".github/actions/setup-conan/**" - ".github/actions/setup-conan/**"
- ".github/scripts/strategy-matrix/**" - ".github/scripts/strategy-matrix/**"
- ".github/workflows/reusable-build.yml"
- ".github/workflows/reusable-build-test-config.yml"
- ".github/workflows/reusable-build-test.yml" - ".github/workflows/reusable-build-test.yml"
- ".github/workflows/reusable-strategy-matrix.yml" - ".github/workflows/reusable-strategy-matrix.yml"
- ".github/workflows/reusable-test.yml"
- ".codecov.yml" - ".codecov.yml"
- "cmake/**" - "cmake/**"
- "conan/**" - "conan/**"
@@ -43,22 +46,8 @@ on:
schedule: schedule:
- cron: "32 6 * * 1-5" - cron: "32 6 * * 1-5"
# Run when manually triggered via the GitHub UI or API. If `force_upload` is # Run when manually triggered via the GitHub UI or API.
# true, then the dependencies that were missing (`force_rebuild` is false) or
# rebuilt (`force_rebuild` is true) will be uploaded, overwriting existing
# dependencies if needed.
workflow_dispatch: workflow_dispatch:
inputs:
dependencies_force_build:
description: "Force building of all dependencies."
required: false
type: boolean
default: false
dependencies_force_upload:
description: "Force uploading of all dependencies."
required: false
type: boolean
default: false
concurrency: concurrency:
group: ${{ github.workflow }}-${{ github.ref }} group: ${{ github.workflow }}-${{ github.ref }}
@@ -82,4 +71,4 @@ jobs:
os: ${{ matrix.os }} os: ${{ matrix.os }}
strategy_matrix: ${{ github.event_name == 'schedule' && 'all' || 'minimal' }} strategy_matrix: ${{ github.event_name == 'schedule' && 'all' || 'minimal' }}
secrets: secrets:
codecov_token: ${{ secrets.CODECOV_TOKEN }} CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }}

View File

@@ -0,0 +1,69 @@
name: Build and test configuration
on:
workflow_call:
inputs:
build_dir:
description: "The directory where to build."
required: true
type: string
build_only:
description: 'Whether to only build or to build and test the code ("true", "false").'
required: true
type: boolean
build_type:
description: 'The build type to use ("Debug", "Release").'
type: string
required: true
cmake_args:
description: "Additional arguments to pass to CMake."
required: false
type: string
default: ""
cmake_target:
description: "The CMake target to build."
type: string
required: true
runs_on:
description: Runner to run the job on as a JSON string
required: true
type: string
image:
description: "The image to run in (leave empty to run natively)"
required: true
type: string
config_name:
description: "The configuration string (used for naming artifacts and such)."
required: true
type: string
secrets:
CODECOV_TOKEN:
description: "The Codecov token to use for uploading coverage reports."
required: true
jobs:
build:
uses: ./.github/workflows/reusable-build.yml
with:
build_dir: ${{ inputs.build_dir }}
build_type: ${{ inputs.build_type }}
cmake_args: ${{ inputs.cmake_args }}
cmake_target: ${{ inputs.cmake_target }}
runs_on: ${{ inputs.runs_on }}
image: ${{ inputs.image }}
config_name: ${{ inputs.config_name }}
secrets:
CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }}
test:
needs: build
uses: ./.github/workflows/reusable-test.yml
with:
run_tests: ${{ !inputs.build_only }}
verify_voidstar: ${{ contains(inputs.cmake_args, '-Dvoidstar=ON') }}
runs_on: ${{ inputs.runs_on }}
image: ${{ inputs.image }}
config_name: ${{ inputs.config_name }}

View File

@@ -13,16 +13,6 @@ on:
required: false required: false
type: string type: string
default: ".build" default: ".build"
dependencies_force_build:
description: "Force building of all dependencies."
required: false
type: boolean
default: false
dependencies_force_upload:
description: "Force uploading of all dependencies."
required: false
type: boolean
default: false
os: os:
description: 'The operating system to use for the build ("linux", "macos", "windows").' description: 'The operating system to use for the build ("linux", "macos", "windows").'
required: true required: true
@@ -34,17 +24,9 @@ on:
type: string type: string
default: "minimal" default: "minimal"
secrets: secrets:
codecov_token: CODECOV_TOKEN:
description: "The Codecov token to use for uploading coverage reports." description: "The Codecov token to use for uploading coverage reports."
required: false required: true
concurrency:
group: ${{ github.workflow }}-${{ github.ref }}-${{ inputs.os }}
cancel-in-progress: true
defaults:
run:
shell: bash
jobs: jobs:
# Generate the strategy matrix to be used by the following job. # Generate the strategy matrix to be used by the following job.
@@ -54,94 +36,23 @@ jobs:
os: ${{ inputs.os }} os: ${{ inputs.os }}
strategy_matrix: ${{ inputs.strategy_matrix }} strategy_matrix: ${{ inputs.strategy_matrix }}
# Build and test the binary. # Build and test the binary for each configuration.
build-test: build-test-config:
needs: needs:
- generate-matrix - generate-matrix
uses: ./.github/workflows/reusable-build-test-config.yml
strategy: strategy:
fail-fast: false fail-fast: false
matrix: ${{ fromJson(needs.generate-matrix.outputs.matrix) }} matrix: ${{ fromJson(needs.generate-matrix.outputs.matrix) }}
max-parallel: 10 max-parallel: 10
runs-on: ${{ matrix.architecture.runner }} with:
container: ${{ inputs.os == 'linux' && format('ghcr.io/xrplf/ci/{0}-{1}:{2}-{3}-sha-5dd7158', matrix.os.distro_name, matrix.os.distro_version, matrix.os.compiler_name, matrix.os.compiler_version) || null }} build_dir: ${{ inputs.build_dir }}
steps: build_only: ${{ matrix.build_only }}
- name: Check strategy matrix build_type: ${{ matrix.build_type }}
run: | cmake_args: ${{ matrix.cmake_args }}
echo 'Operating system distro name: ${{ matrix.os.distro_name }}' cmake_target: ${{ matrix.cmake_target }}
echo 'Operating system distro version: ${{ matrix.os.distro_version }}' runs_on: ${{ toJSON(matrix.architecture.runner) }}
echo 'Operating system compiler name: ${{ matrix.os.compiler_name }}' image: ${{ contains(matrix.architecture.platform, 'linux') && format('ghcr.io/xrplf/ci/{0}-{1}:{2}-{3}-sha-5dd7158', matrix.os.distro_name, matrix.os.distro_version, matrix.os.compiler_name, matrix.os.compiler_version) || '' }}
echo 'Operating system compiler version: ${{ matrix.os.compiler_version }}' config_name: ${{ matrix.config_name }}
echo 'Architecture platform: ${{ matrix.architecture.platform }}' secrets:
echo 'Architecture runner: ${{ toJson(matrix.architecture.runner) }}' CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }}
echo 'Build type: ${{ matrix.build_type }}'
echo 'Build only: ${{ matrix.build_only }}'
echo 'CMake arguments: ${{ matrix.cmake_args }}'
echo 'CMake target: ${{ matrix.cmake_target }}'
echo 'Config name: ${{ matrix.config_name }}'
- name: Cleanup workspace
if: ${{ runner.os == 'macOS' }}
uses: XRPLF/actions/.github/actions/cleanup-workspace@3f044c7478548e3c32ff68980eeb36ece02b364e
- name: Checkout repository
uses: actions/checkout@08eba0b27e820071cde6df949e0beb9ba4906955 # v4.3.0
- name: Prepare runner
uses: XRPLF/actions/.github/actions/prepare-runner@638e0dc11ea230f91bd26622fb542116bb5254d5
with:
disable_ccache: false
- name: Check configuration (Windows)
if: ${{ inputs.os == 'windows' }}
run: |
echo 'Checking environment variables.'
set
echo 'Checking CMake version.'
cmake --version
echo 'Checking Conan version.'
conan --version
- name: Check configuration (Linux and MacOS)
if: ${{ inputs.os == 'linux' || inputs.os == 'macos' }}
run: |
echo 'Checking path.'
echo ${PATH} | tr ':' '\n'
echo 'Checking environment variables.'
env | sort
echo 'Checking CMake version.'
cmake --version
echo 'Checking compiler version.'
${{ inputs.os == 'linux' && '${CC}' || 'clang' }} --version
echo 'Checking Conan version.'
conan --version
echo 'Checking Ninja version.'
ninja --version
echo 'Checking nproc version.'
nproc --version
- name: Setup Conan
uses: ./.github/actions/setup-conan
- name: Build dependencies
uses: ./.github/actions/build-deps
with:
build_dir: ${{ inputs.build_dir }}
build_type: ${{ matrix.build_type }}
force_build: ${{ inputs.dependencies_force_build }}
- name: Build and test binary
uses: ./.github/actions/build-test
with:
build_dir: ${{ inputs.build_dir }}
build_only: ${{ matrix.build_only }}
build_type: ${{ matrix.build_type }}
cmake_args: ${{ matrix.cmake_args }}
cmake_target: ${{ matrix.cmake_target }}
codecov_token: ${{ secrets.codecov_token }}
os: ${{ inputs.os }}

115
.github/workflows/reusable-build.yml vendored Normal file
View File

@@ -0,0 +1,115 @@
name: Build rippled
on:
workflow_call:
inputs:
build_dir:
description: "The directory where to build."
required: true
type: string
build_type:
description: 'The build type to use ("Debug", "Release").'
required: true
type: string
cmake_args:
description: "Additional arguments to pass to CMake."
required: true
type: string
cmake_target:
description: "The CMake target to build."
required: true
type: string
runs_on:
description: Runner to run the job on as a JSON string
required: true
type: string
image:
description: "The image to run in (leave empty to run natively)"
required: true
type: string
config_name:
description: "The name of the configuration."
required: true
type: string
secrets:
CODECOV_TOKEN:
description: "The Codecov token to use for uploading coverage reports."
required: true
defaults:
run:
shell: bash
jobs:
build:
name: Build ${{ inputs.config_name }}
runs-on: ${{ fromJSON(inputs.runs_on) }}
container: ${{ inputs.image != '' && inputs.image || null }}
steps:
- name: Cleanup workspace
if: ${{ runner.os == 'macOS' }}
uses: XRPLF/actions/.github/actions/cleanup-workspace@3f044c7478548e3c32ff68980eeb36ece02b364e
- name: Checkout repository
uses: actions/checkout@08eba0b27e820071cde6df949e0beb9ba4906955 # v4.3.0
- name: Prepare runner
uses: XRPLF/actions/.github/actions/prepare-runner@638e0dc11ea230f91bd26622fb542116bb5254d5
with:
disable_ccache: false
- name: Print build environment
uses: ./.github/actions/print-env
- name: Setup Conan
uses: ./.github/actions/setup-conan
- name: Build dependencies
uses: ./.github/actions/build-deps
with:
build_dir: ${{ inputs.build_dir }}
build_type: ${{ inputs.build_type }}
- name: Configure CMake
shell: bash
working-directory: ${{ inputs.build_dir }}
run: |
cmake \
-G '${{ runner.os == 'Windows' && 'Visual Studio 17 2022' || 'Ninja' }}' \
-DCMAKE_TOOLCHAIN_FILE:FILEPATH=build/generators/conan_toolchain.cmake \
-DCMAKE_BUILD_TYPE=${{ inputs.build_type }} \
${{ inputs.cmake_args }} \
..
- name: Build the binary
shell: bash
working-directory: ${{ inputs.build_dir }}
run: |
cmake \
--build . \
--config ${{ inputs.build_type }} \
--parallel $(nproc) \
--target ${{ inputs.cmake_target }}
- name: Upload rippled artifact
uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2
with:
name: rippled-${{ inputs.config_name }}
path: ${{ inputs.build_dir }}/${{ runner.os == 'Windows' && inputs.build_type || '' }}/rippled${{ runner.os == 'Windows' && '.exe' || '' }}
retention-days: 3
if-no-files-found: error
- name: Upload coverage report
if: ${{ inputs.cmake_target == 'coverage' }}
uses: codecov/codecov-action@18283e04ce6e62d37312384ff67231eb8fd56d24 # v5.4.3
with:
disable_search: true
disable_telem: true
fail_ci_if_error: true
files: ${{ inputs.build_dir }}/coverage.xml
plugins: noop
token: ${{ secrets.CODECOV_TOKEN }}
verbose: true

69
.github/workflows/reusable-test.yml vendored Normal file
View File

@@ -0,0 +1,69 @@
name: Test rippled
on:
workflow_call:
inputs:
verify_voidstar:
description: "Whether to verify the presence of voidstar instrumentation."
required: true
type: boolean
run_tests:
description: "Whether to run unit tests"
required: true
type: boolean
runs_on:
description: Runner to run the job on as a JSON string
required: true
type: string
image:
description: "The image to run in (leave empty to run natively)"
required: true
type: string
config_name:
description: "The name of the configuration."
required: true
type: string
jobs:
test:
name: Test ${{ inputs.config_name }}
runs-on: ${{ fromJSON(inputs.runs_on) }}
container: ${{ inputs.image != '' && inputs.image || null }}
steps:
- name: Download rippled artifact
uses: actions/download-artifact@d3f86a106a0bac45b974a628896c90dbdf5c8093 # v4.3.0
with:
name: rippled-${{ inputs.config_name }}
- name: Make binary executable (Linux and macOS)
shell: bash
if: ${{ runner.os == 'Linux' || runner.os == 'macOS' }}
run: |
chmod +x ./rippled
- name: Check linking (Linux)
if: ${{ runner.os == 'Linux' }}
shell: bash
run: |
ldd ./rippled
if [ "$(ldd ./rippled | grep -E '(libstdc\+\+|libgcc)' | wc -l)" -eq 0 ]; then
echo 'The binary is statically linked.'
else
echo 'The binary is dynamically linked.'
exit 1
fi
- name: Verifying presence of instrumentation
if: ${{ inputs.verify_voidstar }}
shell: bash
run: |
./rippled --version | grep libvoidstar
- name: Test the binary
if: ${{ inputs.run_tests }}
shell: bash
run: |
./rippled --unittest --unittest-jobs $(nproc)
ctest -j $(nproc) --output-on-failure

View File

@@ -146,7 +146,13 @@ LoanSet::checkSign(PreclaimContext const& ctx)
if (!ctx.tx.isFieldPresent(sfCounterpartySignature)) if (!ctx.tx.isFieldPresent(sfCounterpartySignature))
return tesSUCCESS; return tesSUCCESS;
auto const counterSig = ctx.tx.getFieldObject(sfCounterpartySignature); auto const counterSig = ctx.tx.getFieldObject(sfCounterpartySignature);
return Transactor::checkSign(ctx, *counterSigner, counterSig); return Transactor::checkSign(
ctx.view,
ctx.flags,
ctx.parentBatchId,
*counterSigner,
counterSig,
ctx.j);
} }
XRPAmount XRPAmount

View File

@@ -665,14 +665,17 @@ Transactor::apply()
NotTEC NotTEC
Transactor::checkSign( Transactor::checkSign(
PreclaimContext const& ctx, ReadView const& view,
ApplyFlags flags,
std::optional<uint256 const> const& parentBatchId,
AccountID const& idAccount, AccountID const& idAccount,
STObject const& sigObject) STObject const& sigObject,
beast::Journal const j)
{ {
{ {
auto const sle = ctx.view.read(keylet::account(idAccount)); auto const sle = view.read(keylet::account(idAccount));
if (ctx.view.rules().enabled(featureLendingProtocol) && if (view.rules().enabled(featureLendingProtocol) &&
isPseudoAccount(sle)) isPseudoAccount(sle))
// Pseudo-accounts can't sign transactions. This check is gated on // Pseudo-accounts can't sign transactions. This check is gated on
// the Lending Protocol amendment because that's the project it was // the Lending Protocol amendment because that's the project it was
@@ -682,7 +685,7 @@ Transactor::checkSign(
auto const pkSigner = sigObject.getFieldVL(sfSigningPubKey); auto const pkSigner = sigObject.getFieldVL(sfSigningPubKey);
// Ignore signature check on batch inner transactions // Ignore signature check on batch inner transactions
if (ctx.parentBatchId && ctx.view.rules().enabled(featureBatch)) if (parentBatchId && view.rules().enabled(featureBatch))
{ {
// Defensive Check: These values are also checked in Batch::preflight // Defensive Check: These values are also checked in Batch::preflight
if (sigObject.isFieldPresent(sfTxnSignature) || !pkSigner.empty() || if (sigObject.isFieldPresent(sfTxnSignature) || !pkSigner.empty() ||
@@ -693,7 +696,7 @@ Transactor::checkSign(
return tesSUCCESS; return tesSUCCESS;
} }
if ((ctx.flags & tapDRY_RUN) && pkSigner.empty() && if ((flags & tapDRY_RUN) && pkSigner.empty() &&
!sigObject.isFieldPresent(sfSigners)) !sigObject.isFieldPresent(sfSigners))
{ {
// simulate: skip signature validation when neither SigningPubKey nor // simulate: skip signature validation when neither SigningPubKey nor
@@ -703,9 +706,9 @@ Transactor::checkSign(
// If the pk is empty and not simulate or simulate and signers, // If the pk is empty and not simulate or simulate and signers,
// then we must be multi-signing. // then we must be multi-signing.
if (ctx.tx.isFieldPresent(sfSigners)) if (sigObject.isFieldPresent(sfSigners))
{ {
return checkMultiSign(ctx, idAccount, sigObject); return checkMultiSign(view, flags, idAccount, sigObject, j);
} }
// Check Single Sign // Check Single Sign
@@ -714,7 +717,7 @@ Transactor::checkSign(
if (!publicKeyType(makeSlice(pkSigner))) if (!publicKeyType(makeSlice(pkSigner)))
{ {
JLOG(ctx.j.trace()) << "checkSign: signing public key type is unknown"; JLOG(j.trace()) << "checkSign: signing public key type is unknown";
return tefBAD_AUTH; // FIXME: should be better error! return tefBAD_AUTH; // FIXME: should be better error!
} }
@@ -722,11 +725,11 @@ Transactor::checkSign(
auto const idSigner = pkSigner.empty() auto const idSigner = pkSigner.empty()
? idAccount ? idAccount
: calcAccountID(PublicKey(makeSlice(pkSigner))); : calcAccountID(PublicKey(makeSlice(pkSigner)));
auto const sleAccount = ctx.view.read(keylet::account(idAccount)); auto const sleAccount = view.read(keylet::account(idAccount));
if (!sleAccount) if (!sleAccount)
return terNO_ACCOUNT; return terNO_ACCOUNT;
return checkSingleSign(ctx, idSigner, idAccount, sleAccount); return checkSingleSign(view, idSigner, idAccount, sleAccount, j);
} }
NotTEC NotTEC
@@ -735,7 +738,8 @@ Transactor::checkSign(PreclaimContext const& ctx)
auto const idAccount = ctx.tx.isFieldPresent(sfDelegate) auto const idAccount = ctx.tx.isFieldPresent(sfDelegate)
? ctx.tx.getAccountID(sfDelegate) ? ctx.tx.getAccountID(sfDelegate)
: ctx.tx.getAccountID(sfAccount); : ctx.tx.getAccountID(sfAccount);
return checkSign(ctx, idAccount, ctx.tx); return checkSign(
ctx.view, ctx.flags, ctx.parentBatchId, idAccount, ctx.tx, ctx.j);
} }
NotTEC NotTEC
@@ -750,7 +754,8 @@ Transactor::checkBatchSign(PreclaimContext const& ctx)
Blob const& pkSigner = signer.getFieldVL(sfSigningPubKey); Blob const& pkSigner = signer.getFieldVL(sfSigningPubKey);
if (pkSigner.empty()) if (pkSigner.empty())
{ {
if (ret = checkMultiSign(ctx, idAccount, signer); if (ret = checkMultiSign(
ctx.view, ctx.flags, idAccount, signer, ctx.j);
!isTesSuccess(ret)) !isTesSuccess(ret))
return ret; return ret;
} }
@@ -774,7 +779,8 @@ Transactor::checkBatchSign(PreclaimContext const& ctx)
return tesSUCCESS; return tesSUCCESS;
} }
if (ret = checkSingleSign(ctx, idSigner, idAccount, sleAccount); if (ret = checkSingleSign(
ctx.view, idSigner, idAccount, sleAccount, ctx.j);
!isTesSuccess(ret)) !isTesSuccess(ret))
return ret; return ret;
} }
@@ -784,14 +790,15 @@ Transactor::checkBatchSign(PreclaimContext const& ctx)
NotTEC NotTEC
Transactor::checkSingleSign( Transactor::checkSingleSign(
PreclaimContext const& ctx, ReadView const& view,
AccountID const& idSigner, AccountID const& idSigner,
AccountID const& idAccount, AccountID const& idAccount,
std::shared_ptr<SLE const> sleAccount) std::shared_ptr<SLE const> sleAccount,
beast::Journal const j)
{ {
bool const isMasterDisabled = sleAccount->isFlag(lsfDisableMaster); bool const isMasterDisabled = sleAccount->isFlag(lsfDisableMaster);
if (ctx.view.rules().enabled(fixMasterKeyAsRegularKey)) if (view.rules().enabled(fixMasterKeyAsRegularKey))
{ {
// Signed with regular key. // Signed with regular key.
if ((*sleAccount)[~sfRegularKey] == idSigner) if ((*sleAccount)[~sfRegularKey] == idSigner)
@@ -828,16 +835,14 @@ Transactor::checkSingleSign(
else if (sleAccount->isFieldPresent(sfRegularKey)) else if (sleAccount->isFieldPresent(sfRegularKey))
{ {
// Signing key does not match master or regular key. // Signing key does not match master or regular key.
JLOG(ctx.j.trace()) JLOG(j.trace()) << "checkSingleSign: Not authorized to use account.";
<< "checkSingleSign: Not authorized to use account.";
return tefBAD_AUTH; return tefBAD_AUTH;
} }
else else
{ {
// No regular key on account and signing key does not match master key. // No regular key on account and signing key does not match master key.
// FIXME: Why differentiate this case from tefBAD_AUTH? // FIXME: Why differentiate this case from tefBAD_AUTH?
JLOG(ctx.j.trace()) JLOG(j.trace()) << "checkSingleSign: Not authorized to use account.";
<< "checkSingleSign: Not authorized to use account.";
return tefBAD_AUTH_MASTER; return tefBAD_AUTH_MASTER;
} }
@@ -846,17 +851,19 @@ Transactor::checkSingleSign(
NotTEC NotTEC
Transactor::checkMultiSign( Transactor::checkMultiSign(
PreclaimContext const& ctx, ReadView const& view,
ApplyFlags flags,
AccountID const& id, AccountID const& id,
STObject const& sigObject) STObject const& sigObject,
beast::Journal const j)
{ {
// Get id's SignerList and Quorum. // Get id's SignerList and Quorum.
std::shared_ptr<STLedgerEntry const> sleAccountSigners = std::shared_ptr<STLedgerEntry const> sleAccountSigners =
ctx.view.read(keylet::signers(id)); view.read(keylet::signers(id));
// If the signer list doesn't exist the account is not multi-signing. // If the signer list doesn't exist the account is not multi-signing.
if (!sleAccountSigners) if (!sleAccountSigners)
{ {
JLOG(ctx.j.trace()) JLOG(j.trace())
<< "applyTransaction: Invalid: Not a multi-signing account."; << "applyTransaction: Invalid: Not a multi-signing account.";
return tefNOT_MULTI_SIGNING; return tefNOT_MULTI_SIGNING;
} }
@@ -871,7 +878,7 @@ Transactor::checkMultiSign(
"ripple::Transactor::checkMultiSign : signer list ID is 0"); "ripple::Transactor::checkMultiSign : signer list ID is 0");
auto accountSigners = auto accountSigners =
SignerEntries::deserialize(*sleAccountSigners, ctx.j, "ledger"); SignerEntries::deserialize(*sleAccountSigners, j, "ledger");
if (!accountSigners) if (!accountSigners)
return accountSigners.error(); return accountSigners.error();
@@ -895,7 +902,7 @@ Transactor::checkMultiSign(
{ {
if (++iter == accountSigners->end()) if (++iter == accountSigners->end())
{ {
JLOG(ctx.j.trace()) JLOG(j.trace())
<< "applyTransaction: Invalid SigningAccount.Account."; << "applyTransaction: Invalid SigningAccount.Account.";
return tefBAD_SIGNATURE; return tefBAD_SIGNATURE;
} }
@@ -903,7 +910,7 @@ Transactor::checkMultiSign(
if (iter->account != txSignerAcctID) if (iter->account != txSignerAcctID)
{ {
// The SigningAccount is not in the SignerEntries. // The SigningAccount is not in the SignerEntries.
JLOG(ctx.j.trace()) JLOG(j.trace())
<< "applyTransaction: Invalid SigningAccount.Account."; << "applyTransaction: Invalid SigningAccount.Account.";
return tefBAD_SIGNATURE; return tefBAD_SIGNATURE;
} }
@@ -917,13 +924,13 @@ Transactor::checkMultiSign(
// STTx::checkMultiSign // STTx::checkMultiSign
if (!spk.empty() && !publicKeyType(makeSlice(spk))) if (!spk.empty() && !publicKeyType(makeSlice(spk)))
{ {
JLOG(ctx.j.trace()) JLOG(j.trace())
<< "checkMultiSign: signing public key type is unknown"; << "checkMultiSign: signing public key type is unknown";
return tefBAD_SIGNATURE; return tefBAD_SIGNATURE;
} }
XRPL_ASSERT( XRPL_ASSERT(
(ctx.flags & tapDRY_RUN) || !spk.empty(), (flags & tapDRY_RUN) || !spk.empty(),
"ripple::Transactor::checkMultiSign : non-empty signer or " "ripple::Transactor::checkMultiSign : non-empty signer or "
"simulation"); "simulation");
AccountID const signingAcctIDFromPubKey = spk.empty() AccountID const signingAcctIDFromPubKey = spk.empty()
@@ -955,8 +962,7 @@ Transactor::checkMultiSign(
// In any of these cases we need to know whether the account is in // In any of these cases we need to know whether the account is in
// the ledger. Determine that now. // the ledger. Determine that now.
auto const sleTxSignerRoot = auto const sleTxSignerRoot = view.read(keylet::account(txSignerAcctID));
ctx.view.read(keylet::account(txSignerAcctID));
if (signingAcctIDFromPubKey == txSignerAcctID) if (signingAcctIDFromPubKey == txSignerAcctID)
{ {
@@ -969,7 +975,7 @@ Transactor::checkMultiSign(
if (signerAccountFlags & lsfDisableMaster) if (signerAccountFlags & lsfDisableMaster)
{ {
JLOG(ctx.j.trace()) JLOG(j.trace())
<< "applyTransaction: Signer:Account lsfDisableMaster."; << "applyTransaction: Signer:Account lsfDisableMaster.";
return tefMASTER_DISABLED; return tefMASTER_DISABLED;
} }
@@ -981,21 +987,21 @@ Transactor::checkMultiSign(
// Public key must hash to the account's regular key. // Public key must hash to the account's regular key.
if (!sleTxSignerRoot) if (!sleTxSignerRoot)
{ {
JLOG(ctx.j.trace()) << "applyTransaction: Non-phantom signer " JLOG(j.trace()) << "applyTransaction: Non-phantom signer "
"lacks account root."; "lacks account root.";
return tefBAD_SIGNATURE; return tefBAD_SIGNATURE;
} }
if (!sleTxSignerRoot->isFieldPresent(sfRegularKey)) if (!sleTxSignerRoot->isFieldPresent(sfRegularKey))
{ {
JLOG(ctx.j.trace()) JLOG(j.trace())
<< "applyTransaction: Account lacks RegularKey."; << "applyTransaction: Account lacks RegularKey.";
return tefBAD_SIGNATURE; return tefBAD_SIGNATURE;
} }
if (signingAcctIDFromPubKey != if (signingAcctIDFromPubKey !=
sleTxSignerRoot->getAccountID(sfRegularKey)) sleTxSignerRoot->getAccountID(sfRegularKey))
{ {
JLOG(ctx.j.trace()) JLOG(j.trace())
<< "applyTransaction: Account doesn't match RegularKey."; << "applyTransaction: Account doesn't match RegularKey.";
return tefBAD_SIGNATURE; return tefBAD_SIGNATURE;
} }
@@ -1007,8 +1013,7 @@ Transactor::checkMultiSign(
// Cannot perform transaction if quorum is not met. // Cannot perform transaction if quorum is not met.
if (weightSum < sleAccountSigners->getFieldU32(sfSignerQuorum)) if (weightSum < sleAccountSigners->getFieldU32(sfSignerQuorum))
{ {
JLOG(ctx.j.trace()) JLOG(j.trace()) << "applyTransaction: Signers failed to meet quorum.";
<< "applyTransaction: Signers failed to meet quorum.";
return tefBAD_QUORUM; return tefBAD_QUORUM;
} }

View File

@@ -283,9 +283,12 @@ protected:
static NotTEC static NotTEC
checkSign( checkSign(
PreclaimContext const& ctx, ReadView const& view,
AccountID const& id, ApplyFlags flags,
STObject const& sigObject); std::optional<uint256 const> const& parentBatchId,
AccountID const& idAccount,
STObject const& sigObject,
beast::Journal const j);
// Base class always returns true // Base class always returns true
static bool static bool
@@ -335,15 +338,18 @@ private:
payFee(); payFee();
static NotTEC static NotTEC
checkSingleSign( checkSingleSign(
PreclaimContext const& ctx, ReadView const& view,
AccountID const& idSigner, AccountID const& idSigner,
AccountID const& idAccount, AccountID const& idAccount,
std::shared_ptr<SLE const> sleAccount); std::shared_ptr<SLE const> sleAccount,
beast::Journal const j);
static NotTEC static NotTEC
checkMultiSign( checkMultiSign(
PreclaimContext const& ctx, ReadView const& view,
ApplyFlags flags,
AccountID const& id, AccountID const& id,
STObject const& sigObject); STObject const& sigObject,
beast::Journal const j);
void trapTransaction(uint256) const; void trapTransaction(uint256) const;