Merge branch 'develop' into ripple/wamr

.github/actions/build-deps/action.yml

@@ -20,14 +20,18 @@ runs:
   steps:
     - name: Install Conan dependencies
       shell: bash
+      env:
+        BUILD_DIR: ${{ inputs.build_dir }}
+        BUILD_OPTION: ${{ inputs.force_build == 'true' && '*' || 'missing' }}
+        BUILD_TYPE: ${{ inputs.build_type }}
       run: |
         echo 'Installing dependencies.'
-        mkdir -p ${{ inputs.build_dir }}
-        cd ${{ inputs.build_dir }}
+        mkdir -p '${{ env.BUILD_DIR }}'
+        cd '${{ env.BUILD_DIR }}'
         conan install \
           --output-folder . \
-          --build ${{ inputs.force_build == 'true' && '"*"' || 'missing' }} \
-          --options:host '&:tests=True' \
-          --options:host '&:xrpld=True' \
-          --settings:all build_type=${{ inputs.build_type }} \
+          --build=${{ env.BUILD_OPTION }} \
+          --options:host='&:tests=True' \
+          --options:host='&:xrpld=True' \
+          --settings:all build_type='${{ env.BUILD_TYPE }}' \
           ..

.github/actions/build-test/action.yml

@@ -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

.github/actions/print-env/action.yml

@@ -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

.github/actions/setup-conan/action.yml

@@ -35,9 +35,12 @@ runs:
 
     - name: Set up Conan remote
       shell: bash
+      env:
+        CONAN_REMOTE_NAME: ${{ inputs.conan_remote_name }}
+        CONAN_REMOTE_URL: ${{ inputs.conan_remote_url }}
       run: |
-        echo "Adding Conan remote '${{ inputs.conan_remote_name }}' at ${{ inputs.conan_remote_url }}."
-        conan remote add --index 0 --force ${{ inputs.conan_remote_name }} ${{ inputs.conan_remote_url }}
+        echo "Adding Conan remote '${{ env.CONAN_REMOTE_NAME }}' at '${{ env.CONAN_REMOTE_URL }}'."
+        conan remote add --index 0 --force '${{ env.CONAN_REMOTE_NAME }}' '${{ env.CONAN_REMOTE_URL }}'
 
         echo 'Listing Conan remotes.'
         conan remote list

.github/scripts/strategy-matrix/generate.py

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

.github/workflows/on-pr.yml

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

.github/workflows/on-trigger.yml

@@ -23,8 +23,11 @@ on:
       - ".github/actions/build-test/**"
       - ".github/actions/setup-conan/**"
       - ".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-strategy-matrix.yml"
+      - ".github/workflows/reusable-test.yml"
       - ".codecov.yml"
       - "cmake/**"
       - "conan/**"
@@ -43,22 +46,8 @@ on:
   schedule:
     - cron: "32 6 * * 1-5"
 
-  # Run when manually triggered via the GitHub UI or API. If `force_upload` is
-  # 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.
+  # Run when manually triggered via the GitHub UI or API.
   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:
   group: ${{ github.workflow }}-${{ github.ref }}
@@ -82,4 +71,4 @@ jobs:
       os: ${{ matrix.os }}
       strategy_matrix: ${{ github.event_name == 'schedule' && 'all' || 'minimal' }}
     secrets:
-      codecov_token: ${{ secrets.CODECOV_TOKEN }}
+      CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }}

.github/workflows/reusable-build-test-config.yml

@@ -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 }}

.github/workflows/reusable-build-test.yml

@@ -13,16 +13,6 @@ on:
         required: false
         type: string
         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:
         description: 'The operating system to use for the build ("linux", "macos", "windows").'
         required: true
@@ -34,17 +24,9 @@ on:
         type: string
         default: "minimal"
     secrets:
-      codecov_token:
+      CODECOV_TOKEN:
         description: "The Codecov token to use for uploading coverage reports."
-        required: false
-
-concurrency:
-  group: ${{ github.workflow }}-${{ github.ref }}-${{ inputs.os }}
-  cancel-in-progress: true
-
-defaults:
-  run:
-    shell: bash
+        required: true
 
 jobs:
   # Generate the strategy matrix to be used by the following job.
@@ -54,94 +36,23 @@ jobs:
       os: ${{ inputs.os }}
       strategy_matrix: ${{ inputs.strategy_matrix }}
 
-  # Build and test the binary.
-  build-test:
+  # Build and test the binary for each configuration.
+  build-test-config:
     needs:
       - generate-matrix
+    uses: ./.github/workflows/reusable-build-test-config.yml
     strategy:
       fail-fast: false
       matrix: ${{ fromJson(needs.generate-matrix.outputs.matrix) }}
       max-parallel: 10
-    runs-on: ${{ matrix.architecture.runner }}
-    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 }}
-    steps:
-      - name: Check strategy matrix
-        run: |
-          echo 'Operating system distro name: ${{ matrix.os.distro_name }}'
-          echo 'Operating system distro version: ${{ matrix.os.distro_version }}'
-          echo 'Operating system compiler name: ${{ matrix.os.compiler_name }}'
-          echo 'Operating system compiler version: ${{ matrix.os.compiler_version }}'
-          echo 'Architecture platform: ${{ matrix.architecture.platform }}'
-          echo 'Architecture runner: ${{ toJson(matrix.architecture.runner) }}'
-          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 }}
+    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 }}
+      runs_on: ${{ toJSON(matrix.architecture.runner) }}
+      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) || '' }}
+      config_name: ${{ matrix.config_name }}
+    secrets:
+      CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }}

.github/workflows/reusable-build.yml

@@ -0,0 +1,121 @@
+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 }}
+        env:
+          BUILD_TYPE: ${{ inputs.build_type }}
+          CMAKE_ARGS: ${{ inputs.cmake_args }}
+        run: |
+          cmake \
+            -G '${{ runner.os == 'Windows' && 'Visual Studio 17 2022' || 'Ninja' }}' \
+            -DCMAKE_TOOLCHAIN_FILE:FILEPATH=build/generators/conan_toolchain.cmake \
+            -DCMAKE_BUILD_TYPE=${{ env.BUILD_TYPE }} \
+            ${{ env.CMAKE_ARGS }} \
+            ..
+
+      - name: Build the binary
+        shell: bash
+        working-directory: ${{ inputs.build_dir }}
+        env:
+          BUILD_TYPE: ${{ inputs.build_type }}
+          CMAKE_TARGET: ${{ inputs.cmake_target }}
+        run: |
+          cmake \
+            --build . \
+            --config ${{ env.BUILD_TYPE }} \
+            --parallel $(nproc) \
+            --target ${{ env.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

.github/workflows/reusable-notify-clio.yml

@@ -46,41 +46,44 @@ jobs:
         uses: actions/checkout@08eba0b27e820071cde6df949e0beb9ba4906955 # v4.3.0
       - name: Generate outputs
         id: generate
+        env:
+          PR_NUMBER: ${{ github.event.pull_request.number }}
         run: |
           echo 'Generating user and channel.'
           echo "user=clio" >> "${GITHUB_OUTPUT}"
-          echo "channel=pr_${{ github.event.pull_request.number }}" >> "${GITHUB_OUTPUT}"
+          echo "channel=pr_${{ env.PR_NUMBER }}" >> "${GITHUB_OUTPUT}"
           echo 'Extracting version.'
           echo "version=$(cat src/libxrpl/protocol/BuildInfo.cpp | grep "versionString =" | awk -F '"' '{print $2}')" >> "${GITHUB_OUTPUT}"
       - name: Calculate conan reference
         id: conan_ref
         run: |
           echo "conan_ref=${{ steps.generate.outputs.version }}@${{ steps.generate.outputs.user }}/${{ steps.generate.outputs.channel }}" >> "${GITHUB_OUTPUT}"
-
       - name: Set up Conan
         uses: ./.github/actions/setup-conan
         with:
           conan_remote_name: ${{ inputs.conan_remote_name }}
           conan_remote_url: ${{ inputs.conan_remote_url }}
-
       - name: Log into Conan remote
         run: conan remote login ${{ inputs.conan_remote_name }} "${{ secrets.conan_remote_username }}" --password "${{ secrets.conan_remote_password }}"
       - name: Upload package
+        env:
+          CONAN_REMOTE_NAME: ${{ inputs.conan_remote_name }}
         run: |
           conan export --user=${{ steps.generate.outputs.user }} --channel=${{ steps.generate.outputs.channel }} .
-          conan upload --confirm --check --remote=${{ inputs.conan_remote_name }} xrpl/${{ steps.conan_ref.outputs.conan_ref }}
+          conan upload --confirm --check --remote=${{ env.CONAN_REMOTE_NAME }} xrpl/${{ steps.conan_ref.outputs.conan_ref }}
     outputs:
       conan_ref: ${{ steps.conan_ref.outputs.conan_ref }}
 
   notify:
     needs: upload
     runs-on: ubuntu-latest
-    env:
-      GH_TOKEN: ${{ secrets.clio_notify_token }}
     steps:
       - name: Notify Clio
+        env:
+          GH_TOKEN: ${{ secrets.clio_notify_token }}
+          PR_URL: ${{ github.event.pull_request.html_url }}
         run: |
           gh api --method POST -H "Accept: application/vnd.github+json" -H "X-GitHub-Api-Version: 2022-11-28" \
           /repos/xrplf/clio/dispatches -f "event_type=check_libxrpl" \
           -F "client_payload[conan_ref]=${{ needs.upload.outputs.conan_ref }}" \
-          -F "client_payload[pr_url]=${{ github.event.pull_request.html_url }}"
+          -F "client_payload[pr_url]=${{ env.PR_URL }}"

.github/workflows/reusable-strategy-matrix.yml

@@ -35,4 +35,7 @@ jobs:
       - name: Generate strategy matrix
         working-directory: .github/scripts/strategy-matrix
         id: generate
-        run: ./generate.py ${{ inputs.strategy_matrix == 'all' && '--all' || '' }} ${{ inputs.os != '' && format('--config={0}.json', inputs.os) || '' }} >> "${GITHUB_OUTPUT}"
+        env:
+          GENERATE_CONFIG: ${{ inputs.os != '' && format('--config={0}.json', inputs.os) || '' }}
+          GENERATE_OPTION: ${{ inputs.strategy_matrix == 'all' && '--all' || '' }}
+        run: ./generate.py ${{ env.GENERATE_OPTION }} ${{ env.GENERATE_CONFIG }} >> "${GITHUB_OUTPUT}"

.github/workflows/reusable-test.yml

@@ -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

.github/workflows/upload-conan-deps.yml

@@ -24,13 +24,10 @@ on:
     branches: [develop]
     paths:
       - .github/workflows/upload-conan-deps.yml
-
       - .github/workflows/reusable-strategy-matrix.yml
-
       - .github/actions/build-deps/action.yml
       - .github/actions/setup-conan/action.yml
       - ".github/scripts/strategy-matrix/**"
-
       - conanfile.py
       - conan.lock
 
@@ -88,4 +85,6 @@ jobs:
 
       - name: Upload Conan packages
         if: ${{ github.repository_owner == 'XRPLF' && github.event_name != 'pull_request' && github.event_name != 'schedule' }}
-        run: conan upload "*" -r=${{ env.CONAN_REMOTE_NAME }} --confirm ${{ github.event.inputs.force_upload == 'true' && '--force' || '' }}
+        env:
+          FORCE_OPTION: ${{ github.event.inputs.force_upload == 'true' && '--force' || '' }}
+        run: conan upload "*" --remote='${{ env.CONAN_REMOTE_NAME }}' --confirm ${{ env.FORCE_OPTION }}

include/xrpl/protocol/Permissions.h

@@ -86,6 +86,9 @@ public:
     std::optional<TxType>
     getGranularTxType(GranularPermissionType const& gpType) const;
 
+    std::optional<std::reference_wrapper<uint256 const>> const
+    getTxFeature(TxType txType) const;
+
     bool
     isDelegatable(std::uint32_t const& permissionValue, Rules const& rules)
         const;

include/xrpl/protocol/TER.h

@@ -673,7 +673,8 @@ isTerRetry(TER x) noexcept
 inline bool
 isTesSuccess(TER x) noexcept
 {
-    return (x == tesSUCCESS);
+    // Makes use of TERSubset::operator bool()
+    return !(x);
 }
 
 inline bool

include/xrpl/protocol/detail/features.macro

@@ -32,7 +32,7 @@
 // If you add an amendment here, then do not forget to increment `numFeatures`
 // in include/xrpl/protocol/Feature.h.
 
-XRPL_FIX    (IncludeKeyletFields,        Supported::no,  VoteBehavior::DefaultNo)
+XRPL_FIX    (IncludeKeyletFields,        Supported::yes, VoteBehavior::DefaultNo)
 XRPL_FEATURE(DynamicMPT,                 Supported::no,  VoteBehavior::DefaultNo)
 XRPL_FIX    (TokenEscrowV1,              Supported::yes, VoteBehavior::DefaultNo)
 XRPL_FIX    (DelegateV1_1,               Supported::no,  VoteBehavior::DefaultNo)

src/libxrpl/protocol/Permissions.cpp

@@ -147,6 +147,19 @@ Permission::getGranularTxType(GranularPermissionType const& gpType) const
     return std::nullopt;
 }
 
+std::optional<std::reference_wrapper<uint256 const>> const
+Permission::getTxFeature(TxType txType) const
+{
+    auto const txFeaturesIt = txFeatureMap_.find(txType);
+    XRPL_ASSERT(
+        txFeaturesIt != txFeatureMap_.end(),
+        "ripple::Permissions::getTxFeature : tx exists in txFeatureMap_");
+
+    if (txFeaturesIt->second == uint256{})
+        return std::nullopt;
+    return txFeaturesIt->second;
+}
+
 bool
 Permission::isDelegatable(
     std::uint32_t const& permissionValue,
@@ -166,16 +179,12 @@ Permission::isDelegatable(
         if (it == delegatableTx_.end())
             return false;
 
-        auto const txFeaturesIt = txFeatureMap_.find(txType);
-        XRPL_ASSERT(
-            txFeaturesIt != txFeatureMap_.end(),
-            "ripple::Permissions::isDelegatable : tx exists in txFeatureMap_");
+        auto const feature = getTxFeature(txType);
 
         // fixDelegateV1_1: Delegation is only allowed if the required amendment
         // for the transaction is enabled. For transactions that do not require
         // an amendment, delegation is always allowed.
-        if (txFeaturesIt->second != uint256{} &&
-            !rules.enabled(txFeaturesIt->second))
+        if (feature && !rules.enabled(*feature))
             return false;
     }
 

src/test/app/AMM_test.cpp

@@ -3572,7 +3572,7 @@ private:
                     env.current()->rules(),
                     tapNONE,
                     env.journal);
-                auto pf = AMMBid::preflight(pfctx);
+                auto pf = Transactor::invokePreflight<AMMBid>(pfctx);
                 BEAST_EXPECT(pf == temDISABLED);
                 env.app().config().features.insert(featureAMM);
             }
@@ -3587,7 +3587,7 @@ private:
                     env.current()->rules(),
                     tapNONE,
                     env.journal);
-                auto pf = AMMBid::preflight(pfctx);
+                auto pf = Transactor::invokePreflight<AMMBid>(pfctx);
                 BEAST_EXPECT(pf != tesSUCCESS);
             }
 
@@ -3602,7 +3602,7 @@ private:
                     env.current()->rules(),
                     tapNONE,
                     env.journal);
-                auto pf = AMMBid::preflight(pfctx);
+                auto pf = Transactor::invokePreflight<AMMBid>(pfctx);
                 BEAST_EXPECT(pf == temBAD_AMM_TOKENS);
             }
         }

src/test/rpc/AccountSet_test.cpp

@@ -19,6 +19,8 @@
 
 #include <test/jtx.h>
 
+#include <xrpld/app/tx/apply.h>
+
 #include <xrpl/protocol/AmountConversions.h>
 #include <xrpl/protocol/Feature.h>
 #include <xrpl/protocol/Quality.h>
@@ -578,6 +580,32 @@ public:
         env.close();
     }
 
+    void
+    testBadSigningKey()
+    {
+        using namespace test::jtx;
+        testcase("Bad signing key");
+        Env env(*this);
+        Account const alice("alice");
+
+        env.fund(XRP(10000), alice);
+        env.close();
+
+        auto jtx = env.jt(noop("alice"), ter(temBAD_SIGNATURE));
+        if (!BEAST_EXPECT(jtx.stx))
+            return;
+        auto stx = std::make_shared<STTx>(*jtx.stx);
+        stx->at(sfSigningPubKey) = makeSlice(std::string("badkey"));
+
+        env.app().openLedger().modify([&](OpenView& view, beast::Journal j) {
+            auto const result =
+                ripple::apply(env.app(), view, *stx, tapNONE, j);
+            BEAST_EXPECT(result.ter == temBAD_SIGNATURE);
+            BEAST_EXPECT(!result.applied);
+            return result.applied;
+        });
+    }
+
     void
     run() override
     {
@@ -594,6 +622,7 @@ public:
         testRequireAuthWithDir();
         testTransferRate();
         testTicket();
+        testBadSigningKey();
     }
 };
 

src/xrpld/app/tx/detail/AMMBid.cpp

@@ -30,21 +30,15 @@
 
 namespace ripple {
 
+bool
+AMMBid::checkExtraFeatures(PreflightContext const& ctx)
+{
+    return ammEnabled(ctx.rules);
+}
+
 NotTEC
 AMMBid::preflight(PreflightContext const& ctx)
 {
-    if (!ammEnabled(ctx.rules))
-        return temDISABLED;
-
-    if (auto const ret = preflight1(ctx); !isTesSuccess(ret))
-        return ret;
-
-    if (ctx.tx.getFlags() & tfUniversalMask)
-    {
-        JLOG(ctx.j.debug()) << "AMM Bid: invalid flags.";
-        return temINVALID_FLAG;
-    }
-
     if (auto const res = invalidAMMAssetPair(
             ctx.tx[sfAsset].get<Issue>(), ctx.tx[sfAsset2].get<Issue>()))
     {
@@ -95,7 +89,7 @@ AMMBid::preflight(PreflightContext const& ctx)
         }
     }
 
-    return preflight2(ctx);
+    return tesSUCCESS;
 }
 
 TER

src/xrpld/app/tx/detail/AMMBid.h

@@ -71,6 +71,9 @@ public:
     {
     }
 
+    static bool
+    checkExtraFeatures(PreflightContext const& ctx);
+
     static NotTEC
     preflight(PreflightContext const& ctx);
 

src/xrpld/app/tx/detail/AMMClawback.cpp

@@ -33,19 +33,15 @@
 
 namespace ripple {
 
+std::uint32_t
+AMMClawback::getFlagsMask(PreflightContext const& ctx)
+{
+    return tfAMMClawbackMask;
+}
+
 NotTEC
 AMMClawback::preflight(PreflightContext const& ctx)
 {
-    if (!ctx.rules.enabled(featureAMMClawback))
-        return temDISABLED;
-
-    if (auto const ret = preflight1(ctx); !isTesSuccess(ret))
-        return ret;  // LCOV_EXCL_LINE
-
-    auto const flags = ctx.tx.getFlags();
-    if (flags & tfAMMClawbackMask)
-        return temINVALID_FLAG;
-
     AccountID const issuer = ctx.tx[sfAccount];
     AccountID const holder = ctx.tx[sfHolder];
 
@@ -63,6 +59,8 @@ AMMClawback::preflight(PreflightContext const& ctx)
     if (isXRP(asset))
         return temMALFORMED;
 
+    auto const flags = ctx.tx.getFlags();
+
     if (flags & tfClawTwoAssets && asset.account != asset2.account)
     {
         JLOG(ctx.j.trace())
@@ -88,7 +86,7 @@ AMMClawback::preflight(PreflightContext const& ctx)
     if (clawAmount && *clawAmount <= beast::zero)
         return temBAD_AMOUNT;
 
-    return preflight2(ctx);
+    return tesSUCCESS;
 }
 
 TER

src/xrpld/app/tx/detail/AMMClawback.h

@@ -33,6 +33,9 @@ public:
     {
     }
 
+    static std::uint32_t
+    getFlagsMask(PreflightContext const& ctx);
+
     static NotTEC
     preflight(PreflightContext const& ctx);
 

src/xrpld/app/tx/detail/AMMCreate.cpp

@@ -31,21 +31,15 @@
 
 namespace ripple {
 
+bool
+AMMCreate::checkExtraFeatures(PreflightContext const& ctx)
+{
+    return ammEnabled(ctx.rules);
+}
+
 NotTEC
 AMMCreate::preflight(PreflightContext const& ctx)
 {
-    if (!ammEnabled(ctx.rules))
-        return temDISABLED;
-
-    if (auto const ret = preflight1(ctx); !isTesSuccess(ret))
-        return ret;
-
-    if (ctx.tx.getFlags() & tfUniversalMask)
-    {
-        JLOG(ctx.j.debug()) << "AMM Instance: invalid flags.";
-        return temINVALID_FLAG;
-    }
-
     auto const amount = ctx.tx[sfAmount];
     auto const amount2 = ctx.tx[sfAmount2];
 
@@ -74,14 +68,14 @@ AMMCreate::preflight(PreflightContext const& ctx)
         return temBAD_FEE;
     }
 
-    return preflight2(ctx);
+    return tesSUCCESS;
 }
 
 XRPAmount
 AMMCreate::calculateBaseFee(ReadView const& view, STTx const& tx)
 {
     // The fee required for AMMCreate is one owner reserve.
-    return view.fees().increment;
+    return calculateOwnerReserveFee(view, tx);
 }
 
 TER

src/xrpld/app/tx/detail/AMMCreate.h

@@ -63,6 +63,9 @@ public:
     {
     }
 
+    static bool
+    checkExtraFeatures(PreflightContext const& ctx);
+
     static NotTEC
     preflight(PreflightContext const& ctx);
 

src/xrpld/app/tx/detail/AMMDelete.cpp

@@ -27,22 +27,16 @@
 
 namespace ripple {
 
+bool
+AMMDelete::checkExtraFeatures(PreflightContext const& ctx)
+{
+    return ammEnabled(ctx.rules);
+}
+
 NotTEC
 AMMDelete::preflight(PreflightContext const& ctx)
 {
-    if (!ammEnabled(ctx.rules))
-        return temDISABLED;
-
-    if (auto const ret = preflight1(ctx); !isTesSuccess(ret))
-        return ret;
-
-    if (ctx.tx.getFlags() & tfUniversalMask)
-    {
-        JLOG(ctx.j.debug()) << "AMM Delete: invalid flags.";
-        return temINVALID_FLAG;
-    }
-
-    return preflight2(ctx);
+    return tesSUCCESS;
 }
 
 TER

src/xrpld/app/tx/detail/AMMDelete.h

@@ -39,6 +39,9 @@ public:
     {
     }
 
+    static bool
+    checkExtraFeatures(PreflightContext const& ctx);
+
     static NotTEC
     preflight(PreflightContext const& ctx);
 

src/xrpld/app/tx/detail/AMMDeposit.cpp

@@ -29,21 +29,23 @@
 
 namespace ripple {
 
+bool
+AMMDeposit::checkExtraFeatures(PreflightContext const& ctx)
+{
+    return ammEnabled(ctx.rules);
+}
+
+std::uint32_t
+AMMDeposit::getFlagsMask(PreflightContext const& ctx)
+
+{
+    return tfDepositMask;
+}
+
 NotTEC
 AMMDeposit::preflight(PreflightContext const& ctx)
 {
-    if (!ammEnabled(ctx.rules))
-        return temDISABLED;
-
-    if (auto const ret = preflight1(ctx); !isTesSuccess(ret))
-        return ret;
-
     auto const flags = ctx.tx.getFlags();
-    if (flags & tfDepositMask)
-    {
-        JLOG(ctx.j.debug()) << "AMM Deposit: invalid flags.";
-        return temINVALID_FLAG;
-    }
 
     auto const amount = ctx.tx[~sfAmount];
     auto const amount2 = ctx.tx[~sfAmount2];
@@ -159,7 +161,7 @@ AMMDeposit::preflight(PreflightContext const& ctx)
         return temBAD_FEE;
     }
 
-    return preflight2(ctx);
+    return tesSUCCESS;
 }
 
 TER

src/xrpld/app/tx/detail/AMMDeposit.h

@@ -68,6 +68,12 @@ public:
     {
     }
 
+    static bool
+    checkExtraFeatures(PreflightContext const& ctx);
+
+    static std::uint32_t
+    getFlagsMask(PreflightContext const& ctx);
+
     static NotTEC
     preflight(PreflightContext const& ctx);
 

src/xrpld/app/tx/detail/AMMVote.cpp

@@ -27,15 +27,15 @@
 
 namespace ripple {
 
+bool
+AMMVote::checkExtraFeatures(PreflightContext const& ctx)
+{
+    return ammEnabled(ctx.rules);
+}
+
 NotTEC
 AMMVote::preflight(PreflightContext const& ctx)
 {
-    if (!ammEnabled(ctx.rules))
-        return temDISABLED;
-
-    if (auto const ret = preflight1(ctx); !isTesSuccess(ret))
-        return ret;
-
     if (auto const res = invalidAMMAssetPair(
             ctx.tx[sfAsset].get<Issue>(), ctx.tx[sfAsset2].get<Issue>()))
     {
@@ -43,19 +43,13 @@ AMMVote::preflight(PreflightContext const& ctx)
         return res;
     }
 
-    if (ctx.tx.getFlags() & tfUniversalMask)
-    {
-        JLOG(ctx.j.debug()) << "AMM Vote: invalid flags.";
-        return temINVALID_FLAG;
-    }
-
     if (ctx.tx[sfTradingFee] > TRADING_FEE_THRESHOLD)
     {
         JLOG(ctx.j.debug()) << "AMM Vote: invalid trading fee.";
         return temBAD_FEE;
     }
 
-    return preflight2(ctx);
+    return tesSUCCESS;
 }
 
 TER

src/xrpld/app/tx/detail/AMMVote.h

@@ -56,6 +56,9 @@ public:
     {
     }
 
+    static bool
+    checkExtraFeatures(PreflightContext const& ctx);
+
     static NotTEC
     preflight(PreflightContext const& ctx);
 

src/xrpld/app/tx/detail/AMMWithdraw.cpp

@@ -28,21 +28,22 @@
 
 namespace ripple {
 
+bool
+AMMWithdraw::checkExtraFeatures(PreflightContext const& ctx)
+{
+    return ammEnabled(ctx.rules);
+}
+
+std::uint32_t
+AMMWithdraw::getFlagsMask(PreflightContext const& ctx)
+{
+    return tfWithdrawMask;
+}
+
 NotTEC
 AMMWithdraw::preflight(PreflightContext const& ctx)
 {
-    if (!ammEnabled(ctx.rules))
-        return temDISABLED;
-
-    if (auto const ret = preflight1(ctx); !isTesSuccess(ret))
-        return ret;
-
     auto const flags = ctx.tx.getFlags();
-    if (flags & tfWithdrawMask)
-    {
-        JLOG(ctx.j.debug()) << "AMM Withdraw: invalid flags.";
-        return temINVALID_FLAG;
-    }
 
     auto const amount = ctx.tx[~sfAmount];
     auto const amount2 = ctx.tx[~sfAmount2];
@@ -150,7 +151,7 @@ AMMWithdraw::preflight(PreflightContext const& ctx)
         }
     }
 
-    return preflight2(ctx);
+    return tesSUCCESS;
 }
 
 static std::optional<STAmount>

src/xrpld/app/tx/detail/AMMWithdraw.h

@@ -76,6 +76,12 @@ public:
     {
     }
 
+    static bool
+    checkExtraFeatures(PreflightContext const& ctx);
+
+    static std::uint32_t
+    getFlagsMask(PreflightContext const& ctx);
+
     static NotTEC
     preflight(PreflightContext const& ctx);
 

src/xrpld/app/tx/detail/Batch.cpp

@@ -164,6 +164,12 @@ Batch::calculateBaseFee(ReadView const& view, STTx const& tx)
     return signerFees + txnFees + batchBase;
 }
 
+std::uint32_t
+Batch::getFlagsMask(PreflightContext const& ctx)
+{
+    return tfBatchMask;
+}
+
 /**
  * @brief Performs preflight validation checks for a Batch transaction.
  *
@@ -200,23 +206,9 @@ Batch::calculateBaseFee(ReadView const& view, STTx const& tx)
 NotTEC
 Batch::preflight(PreflightContext const& ctx)
 {
-    if (!ctx.rules.enabled(featureBatch))
-        return temDISABLED;
-
-    if (auto const ret = preflight1(ctx); !isTesSuccess(ret))
-        return ret;
-
     auto const parentBatchId = ctx.tx.getTransactionID();
-    auto const outerAccount = ctx.tx.getAccountID(sfAccount);
     auto const flags = ctx.tx.getFlags();
 
-    if (flags & tfBatchMask)
-    {
-        JLOG(ctx.j.debug()) << "BatchTrace[" << parentBatchId << "]:"
-                            << "invalid flags.";
-        return temINVALID_FLAG;
-    }
-
     if (std::popcount(
             flags &
             (tfAllOrNothing | tfOnlyOne | tfUntilFailure | tfIndependent)) != 1)
@@ -242,7 +234,6 @@ Batch::preflight(PreflightContext const& ctx)
     }
 
     // Validation Inner Batch Txns
-    std::unordered_set<AccountID> requiredSigners;
     std::unordered_set<uint256> uniqueHashes;
     std::unordered_map<AccountID, std::unordered_set<std::uint32_t>>
         accountSeqTicket;
@@ -372,6 +363,23 @@ Batch::preflight(PreflightContext const& ctx)
                 }
             }
         }
+    }
+
+    return tesSUCCESS;
+}
+
+NotTEC
+Batch::preflightSigValidated(PreflightContext const& ctx)
+{
+    auto const parentBatchId = ctx.tx.getTransactionID();
+    auto const outerAccount = ctx.tx.getAccountID(sfAccount);
+    auto const& rawTxns = ctx.tx.getFieldArray(sfRawTransactions);
+
+    // Build the signers list
+    std::unordered_set<AccountID> requiredSigners;
+    for (STObject const& rb : rawTxns)
+    {
+        auto const innerAccount = rb.getAccountID(sfAccount);
 
         // If the inner account is the same as the outer account, do not add the
         // inner account to the required signers set.
@@ -379,11 +387,6 @@ Batch::preflight(PreflightContext const& ctx)
             requiredSigners.insert(innerAccount);
     }
 
-    // LCOV_EXCL_START
-    if (auto const ret = preflight2(ctx); !isTesSuccess(ret))
-        return ret;
-    // LCOV_EXCL_STOP
-
     // Validation Batch Signers
     std::unordered_set<AccountID> batchSigners;
     if (ctx.tx.isFieldPresent(sfBatchSigners))

src/xrpld/app/tx/detail/Batch.h

@@ -40,9 +40,15 @@ public:
     static XRPAmount
     calculateBaseFee(ReadView const& view, STTx const& tx);
 
+    static std::uint32_t
+    getFlagsMask(PreflightContext const& ctx);
+
     static NotTEC
     preflight(PreflightContext const& ctx);
 
+    static NotTEC
+    preflightSigValidated(PreflightContext const& ctx);
+
     static NotTEC
     checkSign(PreclaimContext const& ctx);
 

src/xrpld/app/tx/detail/CancelCheck.cpp

@@ -32,21 +32,7 @@ namespace ripple {
 NotTEC
 CancelCheck::preflight(PreflightContext const& ctx)
 {
-    if (!ctx.rules.enabled(featureChecks))
-        return temDISABLED;
-
-    NotTEC const ret{preflight1(ctx)};
-    if (!isTesSuccess(ret))
-        return ret;
-
-    if (ctx.tx.getFlags() & tfUniversalMask)
-    {
-        // There are no flags (other than universal) for CreateCheck yet.
-        JLOG(ctx.j.warn()) << "Malformed transaction: Invalid flags set.";
-        return temINVALID_FLAG;
-    }
-
-    return preflight2(ctx);
+    return tesSUCCESS;
 }
 
 TER

src/xrpld/app/tx/detail/CancelOffer.cpp

@@ -28,25 +28,13 @@ namespace ripple {
 NotTEC
 CancelOffer::preflight(PreflightContext const& ctx)
 {
-    if (auto const ret = preflight1(ctx); !isTesSuccess(ret))
-        return ret;
-
-    auto const uTxFlags = ctx.tx.getFlags();
-
-    if (uTxFlags & tfUniversalMask)
-    {
-        JLOG(ctx.j.trace()) << "Malformed transaction: "
-                            << "Invalid flags set.";
-        return temINVALID_FLAG;
-    }
-
     if (!ctx.tx[sfOfferSequence])
     {
         JLOG(ctx.j.trace()) << "CancelOffer::preflight: missing sequence";
         return temBAD_SEQUENCE;
     }
 
-    return preflight2(ctx);
+    return tesSUCCESS;
 }
 
 //------------------------------------------------------------------------------

src/xrpld/app/tx/detail/CashCheck.cpp

@@ -35,20 +35,6 @@ namespace ripple {
 NotTEC
 CashCheck::preflight(PreflightContext const& ctx)
 {
-    if (!ctx.rules.enabled(featureChecks))
-        return temDISABLED;
-
-    NotTEC const ret{preflight1(ctx)};
-    if (!isTesSuccess(ret))
-        return ret;
-
-    if (ctx.tx.getFlags() & tfUniversalMask)
-    {
-        // There are no flags (other than universal) for CashCheck yet.
-        JLOG(ctx.j.warn()) << "Malformed transaction: Invalid flags set.";
-        return temINVALID_FLAG;
-    }
-
     // Exactly one of Amount or DeliverMin must be present.
     auto const optAmount = ctx.tx[~sfAmount];
     auto const optDeliverMin = ctx.tx[~sfDeliverMin];
@@ -76,7 +62,7 @@ CashCheck::preflight(PreflightContext const& ctx)
         return temBAD_CURRENCY;
     }
 
-    return preflight2(ctx);
+    return tesSUCCESS;
 }
 
 TER

src/xrpld/app/tx/detail/Change.cpp

@@ -33,11 +33,12 @@
 
 namespace ripple {
 
+template <>
 NotTEC
-Change::preflight(PreflightContext const& ctx)
+Transactor::invokePreflight<Change>(PreflightContext const& ctx)
 {
-    auto const ret = preflight0(ctx);
-    if (!isTesSuccess(ret))
+    // 0 means "Allow any flags"
+    if (auto const ret = preflight0(ctx, 0))
         return ret;
 
     auto account = ctx.tx.getAccountID(sfAccount);

src/xrpld/app/tx/detail/Change.h

@@ -33,9 +33,6 @@ public:
     {
     }
 
-    static NotTEC
-    preflight(PreflightContext const& ctx);
-
     TER
     doApply() override;
     void

src/xrpld/app/tx/detail/Clawback.cpp

@@ -75,25 +75,22 @@ preflightHelper<MPTIssue>(PreflightContext const& ctx)
     return tesSUCCESS;
 }
 
+std::uint32_t
+Clawback::getFlagsMask(PreflightContext const& ctx)
+{
+    return tfClawbackMask;
+}
+
 NotTEC
 Clawback::preflight(PreflightContext const& ctx)
 {
-    if (!ctx.rules.enabled(featureClawback))
-        return temDISABLED;
-
-    if (auto const ret = preflight1(ctx); !isTesSuccess(ret))
-        return ret;
-
-    if (ctx.tx.getFlags() & tfClawbackMask)
-        return temINVALID_FLAG;
-
     if (auto const ret = std::visit(
             [&]<typename T>(T const&) { return preflightHelper<T>(ctx); },
             ctx.tx[sfAmount].asset().value());
         !isTesSuccess(ret))
         return ret;
 
-    return preflight2(ctx);
+    return tesSUCCESS;
 }
 
 template <ValidIssueType T>

src/xrpld/app/tx/detail/Clawback.h

@@ -33,6 +33,9 @@ public:
     {
     }
 
+    static std::uint32_t
+    getFlagsMask(PreflightContext const& ctx);
+
     static NotTEC
     preflight(PreflightContext const& ctx);
 

src/xrpld/app/tx/detail/CreateCheck.cpp

@@ -31,19 +31,6 @@ namespace ripple {
 NotTEC
 CreateCheck::preflight(PreflightContext const& ctx)
 {
-    if (!ctx.rules.enabled(featureChecks))
-        return temDISABLED;
-
-    NotTEC const ret{preflight1(ctx)};
-    if (!isTesSuccess(ret))
-        return ret;
-
-    if (ctx.tx.getFlags() & tfUniversalMask)
-    {
-        // There are no flags (other than universal) for CreateCheck yet.
-        JLOG(ctx.j.warn()) << "Malformed transaction: Invalid flags set.";
-        return temINVALID_FLAG;
-    }
     if (ctx.tx[sfAccount] == ctx.tx[sfDestination])
     {
         // They wrote a check to themselves.
@@ -76,7 +63,7 @@ CreateCheck::preflight(PreflightContext const& ctx)
         }
     }
 
-    return preflight2(ctx);
+    return tesSUCCESS;
 }
 
 TER

src/xrpld/app/tx/detail/CreateOffer.cpp

@@ -43,30 +43,36 @@ CreateOffer::makeTxConsequences(PreflightContext const& ctx)
     return TxConsequences{ctx.tx, calculateMaxXRPSpend(ctx.tx)};
 }
 
-NotTEC
-CreateOffer::preflight(PreflightContext const& ctx)
+bool
+CreateOffer::checkExtraFeatures(PreflightContext const& ctx)
 {
     if (ctx.tx.isFieldPresent(sfDomainID) &&
         !ctx.rules.enabled(featurePermissionedDEX))
-        return temDISABLED;
+        return false;
 
-    if (auto const ret = preflight1(ctx); !isTesSuccess(ret))
-        return ret;
+    return true;
+}
 
+std::uint32_t
+CreateOffer::getFlagsMask(PreflightContext const& ctx)
+{
+    // The tfOfferCreateMask is built assuming that PermissionedDEX is
+    // enabled
+    if (ctx.rules.enabled(featurePermissionedDEX))
+        return tfOfferCreateMask;
+    // If PermissionedDEX is not enabled, add tfHybrid to the mask,
+    // indicating it is not allowed.
+    return tfOfferCreateMask | tfHybrid;
+}
+
+NotTEC
+CreateOffer::preflight(PreflightContext const& ctx)
+{
     auto& tx = ctx.tx;
     auto& j = ctx.j;
 
     std::uint32_t const uTxFlags = tx.getFlags();
 
-    if (uTxFlags & tfOfferCreateMask)
-    {
-        JLOG(j.debug()) << "Malformed transaction: Invalid flags set.";
-        return temINVALID_FLAG;
-    }
-
-    if (!ctx.rules.enabled(featurePermissionedDEX) && tx.isFlag(tfHybrid))
-        return temINVALID_FLAG;
-
     if (tx.isFlag(tfHybrid) && !tx.isFieldPresent(sfDomainID))
         return temINVALID_FLAG;
 
@@ -136,7 +142,7 @@ CreateOffer::preflight(PreflightContext const& ctx)
         return temBAD_ISSUER;
     }
 
-    return preflight2(ctx);
+    return tesSUCCESS;
 }
 
 TER

src/xrpld/app/tx/detail/CreateOffer.h

@@ -43,6 +43,12 @@ public:
     static TxConsequences
     makeTxConsequences(PreflightContext const& ctx);
 
+    static bool
+    checkExtraFeatures(PreflightContext const& ctx);
+
+    static std::uint32_t
+    getFlagsMask(PreflightContext const& ctx);
+
     /** Enforce constraints beyond those of the Transactor base class. */
     static NotTEC
     preflight(PreflightContext const& ctx);

src/xrpld/app/tx/detail/CreateTicket.cpp

@@ -36,20 +36,11 @@ CreateTicket::makeTxConsequences(PreflightContext const& ctx)
 NotTEC
 CreateTicket::preflight(PreflightContext const& ctx)
 {
-    if (!ctx.rules.enabled(featureTicketBatch))
-        return temDISABLED;
-
-    if (ctx.tx.getFlags() & tfUniversalMask)
-        return temINVALID_FLAG;
-
     if (std::uint32_t const count = ctx.tx[sfTicketCount];
         count < minValidCount || count > maxValidCount)
         return temINVALID_COUNT;
 
-    if (NotTEC const ret{preflight1(ctx)}; !isTesSuccess(ret))
-        return ret;
-
-    return preflight2(ctx);
+    return tesSUCCESS;
 }
 
 TER

src/xrpld/app/tx/detail/Credentials.cpp

@@ -48,28 +48,19 @@ using namespace credentials;
 
 // ------- CREATE --------------------------
 
+std::uint32_t
+CredentialCreate::getFlagsMask(PreflightContext const& ctx)
+{
+    // 0 means "Allow any flags"
+    return ctx.rules.enabled(fixInvalidTxFlags) ? tfUniversalMask : 0;
+}
+
 NotTEC
 CredentialCreate::preflight(PreflightContext const& ctx)
 {
-    if (!ctx.rules.enabled(featureCredentials))
-    {
-        JLOG(ctx.j.trace()) << "featureCredentials is disabled.";
-        return temDISABLED;
-    }
-
-    if (auto const ret = preflight1(ctx); !isTesSuccess(ret))
-        return ret;
-
     auto const& tx = ctx.tx;
     auto& j = ctx.j;
 
-    if (ctx.rules.enabled(fixInvalidTxFlags) &&
-        (tx.getFlags() & tfUniversalMask))
-    {
-        JLOG(ctx.j.debug()) << "CredentialCreate: invalid flags.";
-        return temINVALID_FLAG;
-    }
-
     if (!tx[sfSubject])
     {
         JLOG(j.trace()) << "Malformed transaction: Invalid Subject";
@@ -91,7 +82,7 @@ CredentialCreate::preflight(PreflightContext const& ctx)
         return temMALFORMED;
     }
 
-    return preflight2(ctx);
+    return tesSUCCESS;
 }
 
 TER
@@ -202,25 +193,17 @@ CredentialCreate::doApply()
 }
 
 // ------- DELETE --------------------------
+
+std::uint32_t
+CredentialDelete::getFlagsMask(PreflightContext const& ctx)
+{
+    // 0 means "Allow any flags"
+    return ctx.rules.enabled(fixInvalidTxFlags) ? tfUniversalMask : 0;
+}
+
 NotTEC
 CredentialDelete::preflight(PreflightContext const& ctx)
 {
-    if (!ctx.rules.enabled(featureCredentials))
-    {
-        JLOG(ctx.j.trace()) << "featureCredentials is disabled.";
-        return temDISABLED;
-    }
-
-    if (auto const ret = preflight1(ctx); !isTesSuccess(ret))
-        return ret;
-
-    if (ctx.rules.enabled(fixInvalidTxFlags) &&
-        (ctx.tx.getFlags() & tfUniversalMask))
-    {
-        JLOG(ctx.j.debug()) << "CredentialDelete: invalid flags.";
-        return temINVALID_FLAG;
-    }
-
     auto const subject = ctx.tx[~sfSubject];
     auto const issuer = ctx.tx[~sfIssuer];
 
@@ -248,7 +231,7 @@ CredentialDelete::preflight(PreflightContext const& ctx)
         return temMALFORMED;
     }
 
-    return preflight2(ctx);
+    return tesSUCCESS;
 }
 
 TER
@@ -289,25 +272,16 @@ CredentialDelete::doApply()
 
 // ------- APPLY --------------------------
 
+std::uint32_t
+CredentialAccept::getFlagsMask(PreflightContext const& ctx)
+{
+    // 0 means "Allow any flags"
+    return ctx.rules.enabled(fixInvalidTxFlags) ? tfUniversalMask : 0;
+}
+
 NotTEC
 CredentialAccept::preflight(PreflightContext const& ctx)
 {
-    if (!ctx.rules.enabled(featureCredentials))
-    {
-        JLOG(ctx.j.trace()) << "featureCredentials is disabled.";
-        return temDISABLED;
-    }
-
-    if (auto const ret = preflight1(ctx); !isTesSuccess(ret))
-        return ret;
-
-    if (ctx.rules.enabled(fixInvalidTxFlags) &&
-        (ctx.tx.getFlags() & tfUniversalMask))
-    {
-        JLOG(ctx.j.debug()) << "CredentialAccept: invalid flags.";
-        return temINVALID_FLAG;
-    }
-
     if (!ctx.tx[sfIssuer])
     {
         JLOG(ctx.j.trace()) << "Malformed transaction: Issuer field zeroed.";
@@ -322,7 +296,7 @@ CredentialAccept::preflight(PreflightContext const& ctx)
         return temMALFORMED;
     }
 
-    return preflight2(ctx);
+    return tesSUCCESS;
 }
 
 TER

src/xrpld/app/tx/detail/Credentials.h

@@ -33,6 +33,9 @@ public:
     {
     }
 
+    static std::uint32_t
+    getFlagsMask(PreflightContext const& ctx);
+
     static NotTEC
     preflight(PreflightContext const& ctx);
 
@@ -54,6 +57,9 @@ public:
     {
     }
 
+    static std::uint32_t
+    getFlagsMask(PreflightContext const& ctx);
+
     static NotTEC
     preflight(PreflightContext const& ctx);
 
@@ -75,6 +81,9 @@ public:
     {
     }
 
+    static std::uint32_t
+    getFlagsMask(PreflightContext const& ctx);
+
     static NotTEC
     preflight(PreflightContext const& ctx);
 

src/xrpld/app/tx/detail/DID.cpp

@@ -45,15 +45,6 @@ namespace ripple {
 NotTEC
 DIDSet::preflight(PreflightContext const& ctx)
 {
-    if (!ctx.rules.enabled(featureDID))
-        return temDISABLED;
-
-    if (ctx.tx.getFlags() & tfUniversalMask)
-        return temINVALID_FLAG;
-
-    if (auto const ret = preflight1(ctx); !isTesSuccess(ret))
-        return ret;
-
     if (!ctx.tx.isFieldPresent(sfURI) &&
         !ctx.tx.isFieldPresent(sfDIDDocument) && !ctx.tx.isFieldPresent(sfData))
         return temEMPTY_DID;
@@ -74,7 +65,7 @@ DIDSet::preflight(PreflightContext const& ctx)
         isTooLong(sfData, maxDIDAttestationLength))
         return temMALFORMED;
 
-    return preflight2(ctx);
+    return tesSUCCESS;
 }
 
 TER
@@ -174,16 +165,7 @@ DIDSet::doApply()
 NotTEC
 DIDDelete::preflight(PreflightContext const& ctx)
 {
-    if (!ctx.rules.enabled(featureDID))
-        return temDISABLED;
-
-    if (ctx.tx.getFlags() & tfUniversalMask)
-        return temINVALID_FLAG;
-
-    if (auto const ret = preflight1(ctx); !isTesSuccess(ret))
-        return ret;
-
-    return preflight2(ctx);
+    return tesSUCCESS;
 }
 
 TER

src/xrpld/app/tx/detail/DelegateSet.cpp

@@ -30,12 +30,6 @@ namespace ripple {
 NotTEC
 DelegateSet::preflight(PreflightContext const& ctx)
 {
-    if (!ctx.rules.enabled(featurePermissionDelegation))
-        return temDISABLED;
-
-    if (auto const ret = preflight1(ctx); !isTesSuccess(ret))
-        return ret;
-
     auto const& permissions = ctx.tx.getFieldArray(sfPermissions);
     if (permissions.size() > permissionMaxSize)
         return temARRAY_TOO_LARGE;
@@ -57,7 +51,7 @@ DelegateSet::preflight(PreflightContext const& ctx)
             return temMALFORMED;
     }
 
-    return preflight2(ctx);
+    return tesSUCCESS;
 }
 
 TER

src/xrpld/app/tx/detail/DeleteAccount.cpp

@@ -38,22 +38,22 @@
 
 namespace ripple {
 
-NotTEC
-DeleteAccount::preflight(PreflightContext const& ctx)
+bool
+DeleteAccount::checkExtraFeatures(PreflightContext const& ctx)
 {
     if (!ctx.rules.enabled(featureDeletableAccounts))
-        return temDISABLED;
+        return false;
 
     if (ctx.tx.isFieldPresent(sfCredentialIDs) &&
         !ctx.rules.enabled(featureCredentials))
-        return temDISABLED;
+        return false;
 
-    if (ctx.tx.getFlags() & tfUniversalMask)
-        return temINVALID_FLAG;
-
-    if (auto const ret = preflight1(ctx); !isTesSuccess(ret))
-        return ret;
+    return true;
+}
 
+NotTEC
+DeleteAccount::preflight(PreflightContext const& ctx)
+{
     if (ctx.tx[sfAccount] == ctx.tx[sfDestination])
         // An account cannot be deleted and give itself the resulting XRP.
         return temDST_IS_SRC;
@@ -62,14 +62,14 @@ DeleteAccount::preflight(PreflightContext const& ctx)
         !isTesSuccess(err))
         return err;
 
-    return preflight2(ctx);
+    return tesSUCCESS;
 }
 
 XRPAmount
 DeleteAccount::calculateBaseFee(ReadView const& view, STTx const& tx)
 {
     // The fee required for AccountDelete is one owner reserve.
-    return view.fees().increment;
+    return calculateOwnerReserveFee(view, tx);
 }
 
 namespace {

src/xrpld/app/tx/detail/DeleteAccount.h

@@ -33,6 +33,9 @@ public:
     {
     }
 
+    static bool
+    checkExtraFeatures(PreflightContext const& ctx);
+
     static NotTEC
     preflight(PreflightContext const& ctx);
 

src/xrpld/app/tx/detail/DeleteOracle.cpp

@@ -29,19 +29,7 @@ namespace ripple {
 NotTEC
 DeleteOracle::preflight(PreflightContext const& ctx)
 {
-    if (!ctx.rules.enabled(featurePriceOracle))
-        return temDISABLED;
-
-    if (auto const ret = preflight1(ctx); !isTesSuccess(ret))
-        return ret;
-
-    if (ctx.tx.getFlags() & tfUniversalMask)
-    {
-        JLOG(ctx.j.debug()) << "Oracle Delete: invalid flags.";
-        return temINVALID_FLAG;
-    }
-
-    return preflight2(ctx);
+    return tesSUCCESS;
 }
 
 TER

src/xrpld/app/tx/detail/DepositPreauth.cpp

@@ -30,32 +30,29 @@
 
 namespace ripple {
 
+bool
+DepositPreauth::checkExtraFeatures(PreflightContext const& ctx)
+{
+    bool const authArrPresent = ctx.tx.isFieldPresent(sfAuthorizeCredentials);
+    bool const unauthArrPresent =
+        ctx.tx.isFieldPresent(sfUnauthorizeCredentials);
+    bool const authCredPresent = authArrPresent || unauthArrPresent;
+
+    if (authCredPresent && !ctx.rules.enabled(featureCredentials))
+        return false;
+
+    return true;
+}
+
 NotTEC
 DepositPreauth::preflight(PreflightContext const& ctx)
 {
-    if (!ctx.rules.enabled(featureDepositPreauth))
-        return temDISABLED;
-
     bool const authArrPresent = ctx.tx.isFieldPresent(sfAuthorizeCredentials);
     bool const unauthArrPresent =
         ctx.tx.isFieldPresent(sfUnauthorizeCredentials);
     int const authCredPresent =
         static_cast<int>(authArrPresent) + static_cast<int>(unauthArrPresent);
 
-    if (authCredPresent && !ctx.rules.enabled(featureCredentials))
-        return temDISABLED;
-
-    if (auto const ret = preflight1(ctx); !isTesSuccess(ret))
-        return ret;
-
-    auto& tx = ctx.tx;
-
-    if (tx.getFlags() & tfUniversalMask)
-    {
-        JLOG(ctx.j.trace()) << "Malformed transaction: Invalid flags set.";
-        return temINVALID_FLAG;
-    }
-
     auto const optAuth = ctx.tx[~sfAuthorize];
     auto const optUnauth = ctx.tx[~sfUnauthorize];
     int const authPresent = static_cast<int>(optAuth.has_value()) +
@@ -102,7 +99,7 @@ DepositPreauth::preflight(PreflightContext const& ctx)
             return err;
     }
 
-    return preflight2(ctx);
+    return tesSUCCESS;
 }
 
 TER

src/xrpld/app/tx/detail/DepositPreauth.h

@@ -33,6 +33,9 @@ public:
     {
     }
 
+    static bool
+    checkExtraFeatures(PreflightContext const& ctx);
+
     static NotTEC
     preflight(PreflightContext const& ctx);
 

src/xrpld/app/tx/detail/Escrow.cpp

@@ -118,15 +118,16 @@ escrowCreatePreflightHelper<MPTIssue>(PreflightContext const& ctx)
     return tesSUCCESS;
 }
 
+std::uint32_t
+EscrowCreate::getFlagsMask(PreflightContext const& ctx)
+{
+    // 0 means "Allow any flags"
+    return ctx.rules.enabled(fix1543) ? tfUniversalMask : 0;
+}
+
 NotTEC
 EscrowCreate::preflight(PreflightContext const& ctx)
 {
-    if (ctx.rules.enabled(fix1543) && ctx.tx.getFlags() & tfUniversalMask)
-        return temINVALID_FLAG;
-
-    if (auto const ret = preflight1(ctx); !isTesSuccess(ret))
-        return ret;
-
     STAmount const amount{ctx.tx[sfAmount]};
     if (!isXRP(amount))
     {
@@ -189,7 +190,7 @@ EscrowCreate::preflight(PreflightContext const& ctx)
             return temDISABLED;
     }
 
-    return preflight2(ctx);
+    return tesSUCCESS;
 }
 
 template <ValidIssueType T>
@@ -629,19 +630,23 @@ checkCondition(Slice f, Slice c)
     return validate(*fulfillment, *condition);
 }
 
+bool
+EscrowFinish::checkExtraFeatures(PreflightContext const& ctx)
+{
+    return !ctx.tx.isFieldPresent(sfCredentialIDs) ||
+        ctx.rules.enabled(featureCredentials);
+}
+
+std::uint32_t
+EscrowFinish::getFlagsMask(PreflightContext const& ctx)
+{
+    // 0 means "Allow any flags"
+    return ctx.rules.enabled(fix1543) ? tfUniversalMask : 0;
+}
+
 NotTEC
 EscrowFinish::preflight(PreflightContext const& ctx)
 {
-    if (ctx.rules.enabled(fix1543) && ctx.tx.getFlags() & tfUniversalMask)
-        return temINVALID_FLAG;
-
-    if (ctx.tx.isFieldPresent(sfCredentialIDs) &&
-        !ctx.rules.enabled(featureCredentials))
-        return temDISABLED;
-
-    if (auto const ret = preflight1(ctx); !isTesSuccess(ret))
-        return ret;
-
     auto const cb = ctx.tx[~sfCondition];
     auto const fb = ctx.tx[~sfFulfillment];
 
@@ -650,13 +655,14 @@ EscrowFinish::preflight(PreflightContext const& ctx)
     if (static_cast<bool>(cb) != static_cast<bool>(fb))
         return temMALFORMED;
 
-    // Verify the transaction signature. If it doesn't work
-    // then don't do any more work.
-    {
-        auto const ret = preflight2(ctx);
-        if (!isTesSuccess(ret))
-            return ret;
-    }
+    return tesSUCCESS;
+}
+
+NotTEC
+EscrowFinish::preflightSigValidated(PreflightContext const& ctx)
+{
+    auto const cb = ctx.tx[~sfCondition];
+    auto const fb = ctx.tx[~sfFulfillment];
 
     if (cb && fb)
     {
@@ -1207,16 +1213,17 @@ EscrowFinish::doApply()
 
 //------------------------------------------------------------------------------
 
+std::uint32_t
+EscrowCancel::getFlagsMask(PreflightContext const& ctx)
+{
+    // 0 means "Allow any flags"
+    return ctx.rules.enabled(fix1543) ? tfUniversalMask : 0;
+}
+
 NotTEC
 EscrowCancel::preflight(PreflightContext const& ctx)
 {
-    if (ctx.rules.enabled(fix1543) && ctx.tx.getFlags() & tfUniversalMask)
-        return temINVALID_FLAG;
-
-    if (auto const ret = preflight1(ctx); !isTesSuccess(ret))
-        return ret;
-
-    return preflight2(ctx);
+    return tesSUCCESS;
 }
 
 template <ValidIssueType T>

src/xrpld/app/tx/detail/Escrow.h

@@ -36,6 +36,9 @@ public:
     static TxConsequences
     makeTxConsequences(PreflightContext const& ctx);
 
+    static std::uint32_t
+    getFlagsMask(PreflightContext const& ctx);
+
     static NotTEC
     preflight(PreflightContext const& ctx);
 
@@ -57,9 +60,18 @@ public:
     {
     }
 
+    static bool
+    checkExtraFeatures(PreflightContext const& ctx);
+
+    static std::uint32_t
+    getFlagsMask(PreflightContext const& ctx);
+
     static NotTEC
     preflight(PreflightContext const& ctx);
 
+    static NotTEC
+    preflightSigValidated(PreflightContext const& ctx);
+
     static XRPAmount
     calculateBaseFee(ReadView const& view, STTx const& tx);
 
@@ -81,6 +93,9 @@ public:
     {
     }
 
+    static std::uint32_t
+    getFlagsMask(PreflightContext const& ctx);
+
     static NotTEC
     preflight(PreflightContext const& ctx);
 

src/xrpld/app/tx/detail/LedgerStateFix.cpp

@@ -30,15 +30,6 @@ namespace ripple {
 NotTEC
 LedgerStateFix::preflight(PreflightContext const& ctx)
 {
-    if (!ctx.rules.enabled(fixNFTokenPageLinks))
-        return temDISABLED;
-
-    if (ctx.tx.getFlags() & tfUniversalMask)
-        return temINVALID_FLAG;
-
-    if (auto const ret = preflight1(ctx); !isTesSuccess(ret))
-        return ret;
-
     switch (ctx.tx[sfLedgerFixType])
     {
         case FixType::nfTokenPageLink:
@@ -50,7 +41,7 @@ LedgerStateFix::preflight(PreflightContext const& ctx)
             return tefINVALID_LEDGER_FIX_TYPE;
     }
 
-    return preflight2(ctx);
+    return tesSUCCESS;
 }
 
 XRPAmount
@@ -58,7 +49,7 @@ LedgerStateFix::calculateBaseFee(ReadView const& view, STTx const& tx)
 {
     // The fee required for LedgerStateFix is one owner reserve, just like
     // the fee for AccountDelete.
-    return view.fees().increment;
+    return calculateOwnerReserveFee(view, tx);
 }
 
 TER

src/xrpld/app/tx/detail/MPTokenAuthorize.cpp

@@ -26,22 +26,19 @@
 
 namespace ripple {
 
+std::uint32_t
+MPTokenAuthorize::getFlagsMask(PreflightContext const& ctx)
+{
+    return tfMPTokenAuthorizeMask;
+}
+
 NotTEC
 MPTokenAuthorize::preflight(PreflightContext const& ctx)
 {
-    if (!ctx.rules.enabled(featureMPTokensV1))
-        return temDISABLED;
-
-    if (auto const ret = preflight1(ctx); !isTesSuccess(ret))
-        return ret;
-
-    if (ctx.tx.getFlags() & tfMPTokenAuthorizeMask)
-        return temINVALID_FLAG;
-
     if (ctx.tx[sfAccount] == ctx.tx[~sfHolder])
         return temMALFORMED;
 
-    return preflight2(ctx);
+    return tesSUCCESS;
 }
 
 TER

src/xrpld/app/tx/detail/MPTokenAuthorize.h

@@ -42,6 +42,9 @@ public:
     {
     }
 
+    static std::uint32_t
+    getFlagsMask(PreflightContext const& ctx);
+
     static NotTEC
     preflight(PreflightContext const& ctx);
 

src/xrpld/app/tx/detail/MPTokenIssuanceCreate.cpp

@@ -25,31 +25,37 @@
 
 namespace ripple {
 
-NotTEC
-MPTokenIssuanceCreate::preflight(PreflightContext const& ctx)
+bool
+MPTokenIssuanceCreate::checkExtraFeatures(PreflightContext const& ctx)
 {
-    if (!ctx.rules.enabled(featureMPTokensV1))
-        return temDISABLED;
-
     if (ctx.tx.isFieldPresent(sfDomainID) &&
         !(ctx.rules.enabled(featurePermissionedDomains) &&
           ctx.rules.enabled(featureSingleAssetVault)))
-        return temDISABLED;
+        return false;
 
     if (ctx.tx.isFieldPresent(sfMutableFlags) &&
         !ctx.rules.enabled(featureDynamicMPT))
-        return temDISABLED;
+        return false;
 
-    if (auto const ret = preflight1(ctx); !isTesSuccess(ret))
-        return ret;
+    return true;
+}
 
+std::uint32_t
+MPTokenIssuanceCreate::getFlagsMask(PreflightContext const& ctx)
+{
+    // This mask is only compared against sfFlags
+    return tfMPTokenIssuanceCreateMask;
+}
+
+NotTEC
+MPTokenIssuanceCreate::preflight(PreflightContext const& ctx)
+{
+    // If the mutable flags field is included, at least one flag must be
+    // specified.
     if (auto const mutableFlags = ctx.tx[~sfMutableFlags]; mutableFlags &&
         (!*mutableFlags || *mutableFlags & tmfMPTokenIssuanceCreateMutableMask))
         return temINVALID_FLAG;
 
-    if (ctx.tx.getFlags() & tfMPTokenIssuanceCreateMask)
-        return temINVALID_FLAG;
-
     if (auto const fee = ctx.tx[~sfTransferFee])
     {
         if (fee > maxTransferFee)
@@ -87,7 +93,7 @@ MPTokenIssuanceCreate::preflight(PreflightContext const& ctx)
         if (maxAmt > maxMPTokenAmount)
             return temMALFORMED;
     }
-    return preflight2(ctx);
+    return tesSUCCESS;
 }
 
 Expected<MPTID, TER>

src/xrpld/app/tx/detail/MPTokenIssuanceCreate.h

@@ -50,6 +50,12 @@ public:
     {
     }
 
+    static bool
+    checkExtraFeatures(PreflightContext const& ctx);
+
+    static std::uint32_t
+    getFlagsMask(PreflightContext const& ctx);
+
     static NotTEC
     preflight(PreflightContext const& ctx);
 

src/xrpld/app/tx/detail/MPTokenIssuanceDestroy.cpp

@@ -25,20 +25,16 @@
 
 namespace ripple {
 
+std::uint32_t
+MPTokenIssuanceDestroy::getFlagsMask(PreflightContext const& ctx)
+{
+    return tfMPTokenIssuanceDestroyMask;
+}
+
 NotTEC
 MPTokenIssuanceDestroy::preflight(PreflightContext const& ctx)
 {
-    if (!ctx.rules.enabled(featureMPTokensV1))
-        return temDISABLED;
-
-    // check flags
-    if (auto const ret = preflight1(ctx); !isTesSuccess(ret))
-        return ret;
-
-    if (ctx.tx.getFlags() & tfMPTokenIssuanceDestroyMask)
-        return temINVALID_FLAG;
-
-    return preflight2(ctx);
+    return tesSUCCESS;
 }
 
 TER

src/xrpld/app/tx/detail/MPTokenIssuanceDestroy.h

@@ -33,6 +33,9 @@ public:
     {
     }
 
+    static std::uint32_t
+    getFlagsMask(PreflightContext const& ctx);
+
     static NotTEC
     preflight(PreflightContext const& ctx);
 

src/xrpld/app/tx/detail/MPTokenIssuanceSet.cpp

@@ -26,6 +26,20 @@
 
 namespace ripple {
 
+bool
+MPTokenIssuanceSet::checkExtraFeatures(PreflightContext const& ctx)
+{
+    return !ctx.tx.isFieldPresent(sfDomainID) ||
+        (ctx.rules.enabled(featurePermissionedDomains) &&
+         ctx.rules.enabled(featureSingleAssetVault));
+}
+
+std::uint32_t
+MPTokenIssuanceSet::getFlagsMask(PreflightContext const& ctx)
+{
+    return tfMPTokenIssuanceSetMask;
+}
+
 // Maps set/clear mutable flags in an MPTokenIssuanceSet transaction to the
 // corresponding ledger mutable flags that control whether the change is
 // allowed.
@@ -49,14 +63,6 @@ static constexpr std::array<MPTMutabilityFlags, 6> mptMutabilityFlags = {
 NotTEC
 MPTokenIssuanceSet::preflight(PreflightContext const& ctx)
 {
-    if (!ctx.rules.enabled(featureMPTokensV1))
-        return temDISABLED;
-
-    if (ctx.tx.isFieldPresent(sfDomainID) &&
-        !(ctx.rules.enabled(featurePermissionedDomains) &&
-          ctx.rules.enabled(featureSingleAssetVault)))
-        return temDISABLED;
-
     auto const mutableFlags = ctx.tx[~sfMutableFlags];
     auto const metadata = ctx.tx[~sfMPTokenMetadata];
     auto const transferFee = ctx.tx[~sfTransferFee];
@@ -68,16 +74,10 @@ MPTokenIssuanceSet::preflight(PreflightContext const& ctx)
     if (ctx.tx.isFieldPresent(sfDomainID) && ctx.tx.isFieldPresent(sfHolder))
         return temMALFORMED;
 
-    if (auto const ret = preflight1(ctx); !isTesSuccess(ret))
-        return ret;
-
     auto const txFlags = ctx.tx.getFlags();
 
-    // check flags
-    if (txFlags & tfMPTokenIssuanceSetMask)
-        return temINVALID_FLAG;
     // fails if both flags are set
-    else if ((txFlags & tfMPTLock) && (txFlags & tfMPTUnlock))
+    if ((txFlags & tfMPTLock) && (txFlags & tfMPTUnlock))
         return temINVALID_FLAG;
 
     auto const accountID = ctx.tx[sfAccount];
@@ -133,7 +133,7 @@ MPTokenIssuanceSet::preflight(PreflightContext const& ctx)
         }
     }
 
-    return preflight2(ctx);
+    return tesSUCCESS;
 }
 
 TER

src/xrpld/app/tx/detail/MPTokenIssuanceSet.h

@@ -33,6 +33,12 @@ public:
     {
     }
 
+    static bool
+    checkExtraFeatures(PreflightContext const& ctx);
+
+    static std::uint32_t
+    getFlagsMask(PreflightContext const& ctx);
+
     static NotTEC
     preflight(PreflightContext const& ctx);
 

src/xrpld/app/tx/detail/NFTokenAcceptOffer.cpp

@@ -27,18 +27,15 @@
 
 namespace ripple {
 
+std::uint32_t
+NFTokenAcceptOffer::getFlagsMask(PreflightContext const& ctx)
+{
+    return tfNFTokenAcceptOfferMask;
+}
+
 NotTEC
 NFTokenAcceptOffer::preflight(PreflightContext const& ctx)
 {
-    if (!ctx.rules.enabled(featureNonFungibleTokensV1))
-        return temDISABLED;
-
-    if (auto const ret = preflight1(ctx); !isTesSuccess(ret))
-        return ret;
-
-    if (ctx.tx.getFlags() & tfNFTokenAcceptOfferMask)
-        return temINVALID_FLAG;
-
     auto const bo = ctx.tx[~sfNFTokenBuyOffer];
     auto const so = ctx.tx[~sfNFTokenSellOffer];
 
@@ -57,7 +54,7 @@ NFTokenAcceptOffer::preflight(PreflightContext const& ctx)
             return temMALFORMED;
     }
 
-    return preflight2(ctx);
+    return tesSUCCESS;
 }
 
 TER

src/xrpld/app/tx/detail/NFTokenAcceptOffer.h

@@ -51,6 +51,9 @@ public:
     {
     }
 
+    static std::uint32_t
+    getFlagsMask(PreflightContext const& ctx);
+
     static NotTEC
     preflight(PreflightContext const& ctx);
 

src/xrpld/app/tx/detail/NFTokenBurn.cpp

@@ -29,16 +29,7 @@ namespace ripple {
 NotTEC
 NFTokenBurn::preflight(PreflightContext const& ctx)
 {
-    if (!ctx.rules.enabled(featureNonFungibleTokensV1))
-        return temDISABLED;
-
-    if (auto const ret = preflight1(ctx); !isTesSuccess(ret))
-        return ret;
-
-    if (ctx.tx.getFlags() & tfUniversalMask)
-        return temINVALID_FLAG;
-
-    return preflight2(ctx);
+    return tesSUCCESS;
 }
 
 TER

src/xrpld/app/tx/detail/NFTokenCancelOffer.cpp

@@ -28,18 +28,15 @@
 
 namespace ripple {
 
+std::uint32_t
+NFTokenCancelOffer::getFlagsMask(PreflightContext const& ctx)
+{
+    return tfNFTokenCancelOfferMask;
+}
+
 NotTEC
 NFTokenCancelOffer::preflight(PreflightContext const& ctx)
 {
-    if (!ctx.rules.enabled(featureNonFungibleTokensV1))
-        return temDISABLED;
-
-    if (auto const ret = preflight1(ctx); !isTesSuccess(ret))
-        return ret;
-
-    if (ctx.tx.getFlags() & tfNFTokenCancelOfferMask)
-        return temINVALID_FLAG;
-
     if (auto const& ids = ctx.tx[sfNFTokenOffers];
         ids.empty() || (ids.size() > maxTokenOfferCancelCount))
         return temMALFORMED;
@@ -51,7 +48,7 @@ NFTokenCancelOffer::preflight(PreflightContext const& ctx)
     if (std::adjacent_find(ids.begin(), ids.end()) != ids.end())
         return temMALFORMED;
 
-    return preflight2(ctx);
+    return tesSUCCESS;
 }
 
 TER

src/xrpld/app/tx/detail/NFTokenCancelOffer.h

@@ -33,6 +33,9 @@ public:
     {
     }
 
+    static std::uint32_t
+    getFlagsMask(PreflightContext const& ctx);
+
     static NotTEC
     preflight(PreflightContext const& ctx);
 

src/xrpld/app/tx/detail/NFTokenCreateOffer.cpp

@@ -26,20 +26,17 @@
 
 namespace ripple {
 
+std::uint32_t
+NFTokenCreateOffer::getFlagsMask(PreflightContext const& ctx)
+{
+    return tfNFTokenCreateOfferMask;
+}
+
 NotTEC
 NFTokenCreateOffer::preflight(PreflightContext const& ctx)
 {
-    if (!ctx.rules.enabled(featureNonFungibleTokensV1))
-        return temDISABLED;
-
-    if (auto const ret = preflight1(ctx); !isTesSuccess(ret))
-        return ret;
-
     auto const txFlags = ctx.tx.getFlags();
 
-    if (txFlags & tfNFTokenCreateOfferMask)
-        return temINVALID_FLAG;
-
     auto const nftFlags = nft::getFlags(ctx.tx[sfNFTokenID]);
 
     // Use implementation shared with NFTokenMint
@@ -55,7 +52,7 @@ NFTokenCreateOffer::preflight(PreflightContext const& ctx)
         !isTesSuccess(notTec))
         return notTec;
 
-    return preflight2(ctx);
+    return tesSUCCESS;
 }
 
 TER

src/xrpld/app/tx/detail/NFTokenCreateOffer.h

@@ -33,6 +33,9 @@ public:
     {
     }
 
+    static std::uint32_t
+    getFlagsMask(PreflightContext const& ctx);
+
     static NotTEC
     preflight(PreflightContext const& ctx);
 

src/xrpld/app/tx/detail/NFTokenMint.cpp

@@ -38,22 +38,23 @@ extractNFTokenFlagsFromTxFlags(std::uint32_t txFlags)
     return static_cast<std::uint16_t>(txFlags & 0x0000FFFF);
 }
 
-NotTEC
-NFTokenMint::preflight(PreflightContext const& ctx)
+static bool
+hasOfferFields(PreflightContext const& ctx)
 {
-    if (!ctx.rules.enabled(featureNonFungibleTokensV1))
-        return temDISABLED;
-
-    bool const hasOfferFields = ctx.tx.isFieldPresent(sfAmount) ||
+    return ctx.tx.isFieldPresent(sfAmount) ||
         ctx.tx.isFieldPresent(sfDestination) ||
         ctx.tx.isFieldPresent(sfExpiration);
+}
 
-    if (!ctx.rules.enabled(featureNFTokenMintOffer) && hasOfferFields)
-        return temDISABLED;
-
-    if (auto const ret = preflight1(ctx); !isTesSuccess(ret))
-        return ret;
+bool
+NFTokenMint::checkExtraFeatures(PreflightContext const& ctx)
+{
+    return ctx.rules.enabled(featureNFTokenMintOffer) || !hasOfferFields(ctx);
+}
 
+std::uint32_t
+NFTokenMint::getFlagsMask(PreflightContext const& ctx)
+{
     // Prior to fixRemoveNFTokenAutoTrustLine, transfer of an NFToken between
     // accounts allowed a TrustLine to be added to the issuer of that token
     // without explicit permission from that issuer.  This was enabled by
@@ -67,7 +68,7 @@ NFTokenMint::preflight(PreflightContext const& ctx)
     // The fixRemoveNFTokenAutoTrustLine amendment disables minting with the
     // tfTrustLine flag as a way to prevent the attack.  But until the
     // amendment passes we still need to keep the old behavior available.
-    std::uint32_t const NFTokenMintMask =
+    std::uint32_t const nfTokenMintMask =
         ctx.rules.enabled(fixRemoveNFTokenAutoTrustLine)
         // if featureDynamicNFT enabled then new flag allowing mutable URI
         // available
@@ -76,9 +77,12 @@ NFTokenMint::preflight(PreflightContext const& ctx)
         : ctx.rules.enabled(featureDynamicNFT) ? tfNFTokenMintOldMaskWithMutable
                                                : tfNFTokenMintOldMask;
 
-    if (ctx.tx.getFlags() & NFTokenMintMask)
-        return temINVALID_FLAG;
+    return nfTokenMintMask;
+}
 
+NotTEC
+NFTokenMint::preflight(PreflightContext const& ctx)
+{
     if (auto const f = ctx.tx[~sfTransferFee])
     {
         if (f > maxTransferFee)
@@ -100,7 +104,7 @@ NFTokenMint::preflight(PreflightContext const& ctx)
             return temMALFORMED;
     }
 
-    if (hasOfferFields)
+    if (hasOfferFields(ctx))
     {
         // The Amount field must be present if either the Destination or
         // Expiration fields are present.
@@ -123,7 +127,7 @@ NFTokenMint::preflight(PreflightContext const& ctx)
         }
     }
 
-    return preflight2(ctx);
+    return tesSUCCESS;
 }
 
 uint256

src/xrpld/app/tx/detail/NFTokenMint.h

@@ -36,6 +36,12 @@ public:
     {
     }
 
+    static bool
+    checkExtraFeatures(PreflightContext const& ctx);
+
+    static std::uint32_t
+    getFlagsMask(PreflightContext const& ctx);
+
     static NotTEC
     preflight(PreflightContext const& ctx);
 

src/xrpld/app/tx/detail/NFTokenModify.cpp

@@ -25,19 +25,15 @@
 
 namespace ripple {
 
+bool
+NFTokenModify::checkExtraFeatures(PreflightContext const& ctx)
+{
+    return ctx.rules.enabled(featureNonFungibleTokensV1_1);
+}
+
 NotTEC
 NFTokenModify::preflight(PreflightContext const& ctx)
 {
-    if (!ctx.rules.enabled(featureNonFungibleTokensV1_1) ||
-        !ctx.rules.enabled(featureDynamicNFT))
-        return temDISABLED;
-
-    if (NotTEC const ret = preflight1(ctx); !isTesSuccess(ret))
-        return ret;
-
-    if (ctx.tx.getFlags() & tfUniversalMask)
-        return temINVALID_FLAG;
-
     if (auto owner = ctx.tx[~sfOwner]; owner == ctx.tx[sfAccount])
         return temMALFORMED;
 
@@ -47,7 +43,7 @@ NFTokenModify::preflight(PreflightContext const& ctx)
             return temMALFORMED;
     }
 
-    return preflight2(ctx);
+    return tesSUCCESS;
 }
 
 TER

src/xrpld/app/tx/detail/NFTokenModify.h

@@ -33,6 +33,9 @@ public:
     {
     }
 
+    static bool
+    checkExtraFeatures(PreflightContext const& ctx);
+
     static NotTEC
     preflight(PreflightContext const& ctx);
 

src/xrpld/app/tx/detail/PayChan.cpp

@@ -171,15 +171,16 @@ PayChanCreate::makeTxConsequences(PreflightContext const& ctx)
     return TxConsequences{ctx.tx, ctx.tx[sfAmount].xrp()};
 }
 
+std::uint32_t
+PayChanCreate::getFlagsMask(PreflightContext const& ctx)
+{
+    // 0 means "Allow any flags"
+    return ctx.rules.enabled(fix1543) ? tfUniversalMask : 0;
+}
+
 NotTEC
 PayChanCreate::preflight(PreflightContext const& ctx)
 {
-    if (ctx.rules.enabled(fix1543) && ctx.tx.getFlags() & tfUniversalMask)
-        return temINVALID_FLAG;
-
-    if (auto const ret = preflight1(ctx); !isTesSuccess(ret))
-        return ret;
-
     if (!isXRP(ctx.tx[sfAmount]) || (ctx.tx[sfAmount] <= beast::zero))
         return temBAD_AMOUNT;
 
@@ -189,7 +190,7 @@ PayChanCreate::preflight(PreflightContext const& ctx)
     if (!publicKeyType(ctx.tx[sfPublicKey]))
         return temMALFORMED;
 
-    return preflight2(ctx);
+    return tesSUCCESS;
 }
 
 TER
@@ -330,19 +331,20 @@ PayChanFund::makeTxConsequences(PreflightContext const& ctx)
     return TxConsequences{ctx.tx, ctx.tx[sfAmount].xrp()};
 }
 
+std::uint32_t
+PayChanFund::getFlagsMask(PreflightContext const& ctx)
+{
+    // 0 means "Allow any flags"
+    return ctx.rules.enabled(fix1543) ? tfUniversalMask : 0;
+}
+
 NotTEC
 PayChanFund::preflight(PreflightContext const& ctx)
 {
-    if (ctx.rules.enabled(fix1543) && ctx.tx.getFlags() & tfUniversalMask)
-        return temINVALID_FLAG;
-
-    if (auto const ret = preflight1(ctx); !isTesSuccess(ret))
-        return ret;
-
     if (!isXRP(ctx.tx[sfAmount]) || (ctx.tx[sfAmount] <= beast::zero))
         return temBAD_AMOUNT;
 
-    return preflight2(ctx);
+    return tesSUCCESS;
 }
 
 TER
@@ -420,16 +422,23 @@ PayChanFund::doApply()
 
 //------------------------------------------------------------------------------
 
+bool
+PayChanClaim::checkExtraFeatures(PreflightContext const& ctx)
+{
+    return !ctx.tx.isFieldPresent(sfCredentialIDs) ||
+        ctx.rules.enabled(featureCredentials);
+}
+
+std::uint32_t
+PayChanClaim::getFlagsMask(PreflightContext const& ctx)
+{
+    // 0 means "Allow any flags"
+    return ctx.rules.enabled(fix1543) ? tfPayChanClaimMask : 0;
+}
+
 NotTEC
 PayChanClaim::preflight(PreflightContext const& ctx)
 {
-    if (ctx.tx.isFieldPresent(sfCredentialIDs) &&
-        !ctx.rules.enabled(featureCredentials))
-        return temDISABLED;
-
-    if (auto const ret = preflight1(ctx); !isTesSuccess(ret))
-        return ret;
-
     auto const bal = ctx.tx[~sfBalance];
     if (bal && (!isXRP(*bal) || *bal <= beast::zero))
         return temBAD_AMOUNT;
@@ -444,9 +453,6 @@ PayChanClaim::preflight(PreflightContext const& ctx)
     {
         auto const flags = ctx.tx.getFlags();
 
-        if (ctx.rules.enabled(fix1543) && (flags & tfPayChanClaimMask))
-            return temINVALID_FLAG;
-
         if ((flags & tfClose) && (flags & tfRenew))
             return temMALFORMED;
     }
@@ -481,7 +487,7 @@ PayChanClaim::preflight(PreflightContext const& ctx)
         !isTesSuccess(err))
         return err;
 
-    return preflight2(ctx);
+    return tesSUCCESS;
 }
 
 TER

src/xrpld/app/tx/detail/PayChan.h

@@ -36,6 +36,9 @@ public:
     static TxConsequences
     makeTxConsequences(PreflightContext const& ctx);
 
+    static std::uint32_t
+    getFlagsMask(PreflightContext const& ctx);
+
     static NotTEC
     preflight(PreflightContext const& ctx);
 
@@ -62,6 +65,9 @@ public:
     static TxConsequences
     makeTxConsequences(PreflightContext const& ctx);
 
+    static std::uint32_t
+    getFlagsMask(PreflightContext const& ctx);
+
     static NotTEC
     preflight(PreflightContext const& ctx);
 
@@ -82,6 +88,12 @@ public:
     {
     }
 
+    static bool
+    checkExtraFeatures(PreflightContext const& ctx);
+
+    static std::uint32_t
+    getFlagsMask(PreflightContext const& ctx);
+
     static NotTEC
     preflight(PreflightContext const& ctx);
 

src/xrpld/app/tx/detail/Payment.cpp

@@ -65,20 +65,33 @@ getMaxSourceAmount(
             dstAmount < beast::zero);
 }
 
-NotTEC
-Payment::preflight(PreflightContext const& ctx)
+bool
+Payment::checkExtraFeatures(PreflightContext const& ctx)
 {
     if (ctx.tx.isFieldPresent(sfCredentialIDs) &&
         !ctx.rules.enabled(featureCredentials))
-        return temDISABLED;
-
+        return false;
     if (ctx.tx.isFieldPresent(sfDomainID) &&
         !ctx.rules.enabled(featurePermissionedDEX))
-        return temDISABLED;
+        return false;
 
-    if (auto const ret = preflight1(ctx); !isTesSuccess(ret))
-        return ret;
+    return true;
+}
 
+std::uint32_t
+Payment::getFlagsMask(PreflightContext const& ctx)
+{
+    auto& tx = ctx.tx;
+
+    STAmount const dstAmount(tx.getFieldAmount(sfAmount));
+    bool const mptDirect = dstAmount.holds<MPTIssue>();
+
+    return mptDirect ? tfMPTPaymentMask : tfPaymentMask;
+}
+
+NotTEC
+Payment::preflight(PreflightContext const& ctx)
+{
     auto& tx = ctx.tx;
     auto& j = ctx.j;
 
@@ -90,14 +103,6 @@ Payment::preflight(PreflightContext const& ctx)
 
     std::uint32_t const txFlags = tx.getFlags();
 
-    std::uint32_t paymentMask = mptDirect ? tfMPTPaymentMask : tfPaymentMask;
-
-    if (txFlags & paymentMask)
-    {
-        JLOG(j.trace()) << "Malformed transaction: Invalid flags set.";
-        return temINVALID_FLAG;
-    }
-
     if (mptDirect && ctx.tx.isFieldPresent(sfPaths))
         return temMALFORMED;
 
@@ -242,7 +247,7 @@ Payment::preflight(PreflightContext const& ctx)
         !isTesSuccess(err))
         return err;
 
-    return preflight2(ctx);
+    return tesSUCCESS;
 }
 
 TER

src/xrpld/app/tx/detail/Payment.h

@@ -42,6 +42,12 @@ public:
     static TxConsequences
     makeTxConsequences(PreflightContext const& ctx);
 
+    static bool
+    checkExtraFeatures(PreflightContext const& ctx);
+
+    static std::uint32_t
+    getFlagsMask(PreflightContext const& ctx);
+
     static NotTEC
     preflight(PreflightContext const& ctx);
 

src/xrpld/app/tx/detail/PermissionedDomainDelete.cpp

@@ -27,23 +27,11 @@ namespace ripple {
 NotTEC
 PermissionedDomainDelete::preflight(PreflightContext const& ctx)
 {
-    if (!ctx.rules.enabled(featurePermissionedDomains))
-        return temDISABLED;
-
-    if (auto const ret = preflight1(ctx); !isTesSuccess(ret))
-        return ret;
-
-    if (ctx.tx.getFlags() & tfUniversalMask)
-    {
-        JLOG(ctx.j.debug()) << "PermissionedDomainDelete: invalid flags.";
-        return temINVALID_FLAG;
-    }
-
     auto const domain = ctx.tx.getFieldH256(sfDomainID);
     if (domain == beast::zero)
         return temMALFORMED;
 
-    return preflight2(ctx);
+    return tesSUCCESS;
 }
 
 TER

src/xrpld/app/tx/detail/PermissionedDomainSet.cpp

@@ -28,22 +28,15 @@
 
 namespace ripple {
 
+bool
+PermissionedDomainSet::checkExtraFeatures(PreflightContext const& ctx)
+{
+    return ctx.rules.enabled(featureCredentials);
+}
+
 NotTEC
 PermissionedDomainSet::preflight(PreflightContext const& ctx)
 {
-    if (!ctx.rules.enabled(featurePermissionedDomains) ||
-        !ctx.rules.enabled(featureCredentials))
-        return temDISABLED;
-
-    if (auto const ret = preflight1(ctx); !isTesSuccess(ret))
-        return ret;
-
-    if (ctx.tx.getFlags() & tfUniversalMask)
-    {
-        JLOG(ctx.j.debug()) << "PermissionedDomainSet: invalid flags.";
-        return temINVALID_FLAG;
-    }
-
     if (auto err = credentials::checkArray(
             ctx.tx.getFieldArray(sfAcceptedCredentials),
             maxPermissionedDomainCredentialsArraySize,
@@ -55,7 +48,7 @@ PermissionedDomainSet::preflight(PreflightContext const& ctx)
     if (domain && *domain == beast::zero)
         return temMALFORMED;
 
-    return preflight2(ctx);
+    return tesSUCCESS;
 }
 
 TER

src/xrpld/app/tx/detail/PermissionedDomainSet.h

@@ -33,6 +33,9 @@ public:
     {
     }
 
+    static bool
+    checkExtraFeatures(PreflightContext const& ctx);
+
     static NotTEC
     preflight(PreflightContext const& ctx);
 

src/xrpld/app/tx/detail/SetAccount.cpp

@@ -57,23 +57,20 @@ SetAccount::makeTxConsequences(PreflightContext const& ctx)
     return TxConsequences{ctx.tx, getTxConsequencesCategory(ctx.tx)};
 }
 
+std::uint32_t
+SetAccount::getFlagsMask(PreflightContext const& ctx)
+{
+    return tfAccountSetMask;
+}
+
 NotTEC
 SetAccount::preflight(PreflightContext const& ctx)
 {
-    if (auto const ret = preflight1(ctx); !isTesSuccess(ret))
-        return ret;
-
     auto& tx = ctx.tx;
     auto& j = ctx.j;
 
     std::uint32_t const uTxFlags = tx.getFlags();
 
-    if (uTxFlags & tfAccountSetMask)
-    {
-        JLOG(j.trace()) << "Malformed transaction: Invalid flags set.";
-        return temINVALID_FLAG;
-    }
-
     std::uint32_t const uSetFlag = tx.getFieldU32(sfSetFlag);
     std::uint32_t const uClearFlag = tx.getFieldU32(sfClearFlag);
 
@@ -186,7 +183,7 @@ SetAccount::preflight(PreflightContext const& ctx)
             return temMALFORMED;
     }
 
-    return preflight2(ctx);
+    return tesSUCCESS;
 }
 
 TER

src/xrpld/app/tx/detail/SetAccount.h

@@ -38,6 +38,9 @@ public:
     static TxConsequences
     makeTxConsequences(PreflightContext const& ctx);
 
+    static std::uint32_t
+    getFlagsMask(PreflightContext const& ctx);
+
     static NotTEC
     preflight(PreflightContext const& ctx);
 

src/xrpld/app/tx/detail/SetOracle.cpp

@@ -39,15 +39,6 @@ tokenPairKey(STObject const& pair)
 NotTEC
 SetOracle::preflight(PreflightContext const& ctx)
 {
-    if (!ctx.rules.enabled(featurePriceOracle))
-        return temDISABLED;
-
-    if (auto const ret = preflight1(ctx); !isTesSuccess(ret))
-        return ret;
-
-    if (ctx.tx.getFlags() & tfUniversalMask)
-        return temINVALID_FLAG;
-
     auto const& dataSeries = ctx.tx.getFieldArray(sfPriceDataSeries);
     if (dataSeries.empty())
         return temARRAY_EMPTY;
@@ -64,7 +55,7 @@ SetOracle::preflight(PreflightContext const& ctx)
         isInvalidLength(sfAssetClass, maxOracleSymbolClass))
         return temMALFORMED;
 
-    return preflight2(ctx);
+    return tesSUCCESS;
 }
 
 TER

src/xrpld/app/tx/detail/SetRegularKey.cpp

@@ -51,18 +51,6 @@ SetRegularKey::calculateBaseFee(ReadView const& view, STTx const& tx)
 NotTEC
 SetRegularKey::preflight(PreflightContext const& ctx)
 {
-    if (auto const ret = preflight1(ctx); !isTesSuccess(ret))
-        return ret;
-
-    std::uint32_t const uTxFlags = ctx.tx.getFlags();
-
-    if (uTxFlags & tfUniversalMask)
-    {
-        JLOG(ctx.j.trace()) << "Malformed transaction: Invalid flags set.";
-
-        return temINVALID_FLAG;
-    }
-
     if (ctx.rules.enabled(fixMasterKeyAsRegularKey) &&
         ctx.tx.isFieldPresent(sfRegularKey) &&
         (ctx.tx.getAccountID(sfRegularKey) == ctx.tx.getAccountID(sfAccount)))
@@ -70,7 +58,7 @@ SetRegularKey::preflight(PreflightContext const& ctx)
         return temBAD_REGKEY;
     }
 
-    return preflight2(ctx);
+    return tesSUCCESS;
 }
 
 TER

src/xrpld/app/tx/detail/SetSignerList.cpp

@@ -77,19 +77,16 @@ SetSignerList::determineOperation(
     return std::make_tuple(tesSUCCESS, quorum, sign, op);
 }
 
+std::uint32_t
+SetSignerList::getFlagsMask(PreflightContext const& ctx)
+{
+    // 0 means "Allow any flags"
+    return ctx.rules.enabled(fixInvalidTxFlags) ? tfUniversalMask : 0;
+}
+
 NotTEC
 SetSignerList::preflight(PreflightContext const& ctx)
 {
-    if (auto const ret = preflight1(ctx); !isTesSuccess(ret))
-        return ret;
-
-    if (ctx.rules.enabled(fixInvalidTxFlags) &&
-        (ctx.tx.getFlags() & tfUniversalMask))
-    {
-        JLOG(ctx.j.debug()) << "SetSignerList: invalid flags.";
-        return temINVALID_FLAG;
-    }
-
     auto const result = determineOperation(ctx.tx, ctx.flags, ctx.j);
 
     if (std::get<0>(result) != tesSUCCESS)
@@ -119,7 +116,7 @@ SetSignerList::preflight(PreflightContext const& ctx)
         }
     }
 
-    return preflight2(ctx);
+    return tesSUCCESS;
 }
 
 TER

src/xrpld/app/tx/detail/SetSignerList.h

@@ -51,6 +51,9 @@ public:
     {
     }
 
+    static std::uint32_t
+    getFlagsMask(PreflightContext const& ctx);
+
     static NotTEC
     preflight(PreflightContext const& ctx);
 

src/xrpld/app/tx/detail/SetTrust.cpp

@@ -67,23 +67,20 @@ computeFreezeFlags(
 
 namespace ripple {
 
+std::uint32_t
+SetTrust::getFlagsMask(PreflightContext const& ctx)
+{
+    return tfTrustSetMask;
+}
+
 NotTEC
 SetTrust::preflight(PreflightContext const& ctx)
 {
-    if (auto const ret = preflight1(ctx); !isTesSuccess(ret))
-        return ret;
-
     auto& tx = ctx.tx;
     auto& j = ctx.j;
 
     std::uint32_t const uTxFlags = tx.getFlags();
 
-    if (uTxFlags & tfTrustSetMask)
-    {
-        JLOG(j.trace()) << "Malformed transaction: Invalid flags set.";
-        return temINVALID_FLAG;
-    }
-
     if (!ctx.rules.enabled(featureDeepFreeze))
     {
         // Even though the deep freeze flags are included in the
@@ -127,7 +124,7 @@ SetTrust::preflight(PreflightContext const& ctx)
         return temDST_NEEDED;
     }
 
-    return preflight2(ctx);
+    return tesSUCCESS;
 }
 
 TER

src/xrpld/app/tx/detail/SetTrust.h

@@ -35,6 +35,9 @@ public:
     {
     }
 
+    static std::uint32_t
+    getFlagsMask(PreflightContext const& ctx);
+
     static NotTEC
     preflight(PreflightContext const& ctx);
 

src/xrpld/app/tx/detail/Transactor.cpp

@@ -41,7 +41,7 @@ namespace ripple {
 
 /** Performs early sanity checks on the txid */
 NotTEC
-preflight0(PreflightContext const& ctx)
+preflight0(PreflightContext const& ctx, std::uint32_t flagMask)
 {
     if (isPseudoTx(ctx.tx) && ctx.tx.isFlag(tfInnerBatchTxn))
     {
@@ -83,12 +83,84 @@ preflight0(PreflightContext const& ctx)
         return temINVALID;
     }
 
+    if (ctx.tx.getFlags() & flagMask)
+    {
+        JLOG(ctx.j.debug())
+            << ctx.tx.peekAtField(sfTransactionType).getFullText()
+            << ": invalid flags.";
+        return temINVALID_FLAG;
+    }
+
     return tesSUCCESS;
 }
 
+namespace detail {
+
+/** Checks the validity of the transactor signing key.
+ *
+ * Normally called from preflight1.
+ */
+NotTEC
+preflightCheckSigningKey(STObject const& sigObject, beast::Journal j)
+{
+    if (auto const spk = sigObject.getFieldVL(sfSigningPubKey);
+        !spk.empty() && !publicKeyType(makeSlice(spk)))
+    {
+        JLOG(j.debug()) << "preflightCheckSigningKey: invalid signing key";
+        return temBAD_SIGNATURE;
+    }
+    return tesSUCCESS;
+}
+
+std::optional<NotTEC>
+preflightCheckSimulateKeys(
+    ApplyFlags flags,
+    STObject const& sigObject,
+    beast::Journal j)
+{
+    if (flags & tapDRY_RUN)  // simulation
+    {
+        std::optional<Slice> const signature = sigObject[~sfTxnSignature];
+        if (signature && !signature->empty())
+        {
+            // NOTE: This code should never be hit because it's checked in the
+            // `simulate` RPC
+            return temINVALID;  // LCOV_EXCL_LINE
+        }
+
+        if (!sigObject.isFieldPresent(sfSigners))
+        {
+            // no signers, no signature - a valid simulation
+            return tesSUCCESS;
+        }
+
+        for (auto const& signer : sigObject.getFieldArray(sfSigners))
+        {
+            if (signer.isFieldPresent(sfTxnSignature) &&
+                !signer[sfTxnSignature].empty())
+            {
+                // NOTE: This code should never be hit because it's
+                // checked in the `simulate` RPC
+                return temINVALID;  // LCOV_EXCL_LINE
+            }
+        }
+
+        Slice const signingPubKey = sigObject[sfSigningPubKey];
+        if (!signingPubKey.empty())
+        {
+            // trying to single-sign _and_ multi-sign a transaction
+            return temINVALID;
+        }
+        return tesSUCCESS;
+    }
+    return {};
+}
+
+}  // namespace detail
+
 /** Performs early sanity checks on the account and fee fields */
 NotTEC
-preflight1(PreflightContext const& ctx)
+Transactor::preflight1(PreflightContext const& ctx, std::uint32_t flagMask)
 {
     // This is inappropriate in preflight0, because only Change transactions
     // skip this function, and those do not allow an sfTicketSequence field.
@@ -107,8 +179,7 @@ preflight1(PreflightContext const& ctx)
             return temBAD_SIGNER;
     }
 
-    auto const ret = preflight0(ctx);
-    if (!isTesSuccess(ret))
+    if (auto const ret = preflight0(ctx, flagMask))
         return ret;
 
     auto const id = ctx.tx.getAccountID(sfAccount);
@@ -126,13 +197,8 @@ preflight1(PreflightContext const& ctx)
         return temBAD_FEE;
     }
 
-    auto const spk = ctx.tx.getSigningPubKey();
-
-    if (!spk.empty() && !publicKeyType(makeSlice(spk)))
-    {
-        JLOG(ctx.j.debug()) << "preflight1: invalid signing key";
-        return temBAD_SIGNATURE;
-    }
+    if (auto const ret = detail::preflightCheckSigningKey(ctx.tx, ctx.j))
+        return ret;
 
     // An AccountTxnID field constrains transaction ordering more than the
     // Sequence field.  Tickets, on the other hand, reduce ordering
@@ -157,41 +223,13 @@ preflight1(PreflightContext const& ctx)
 
 /** Checks whether the signature appears valid */
 NotTEC
-preflight2(PreflightContext const& ctx)
+Transactor::preflight2(PreflightContext const& ctx)
 {
-    if (ctx.flags & tapDRY_RUN)  // simulation
-    {
-        if (!ctx.tx.getSignature().empty())
-        {
-            // NOTE: This code should never be hit because it's checked in the
-            // `simulate` RPC
-            return temINVALID;  // LCOV_EXCL_LINE
-        }
-
-        if (!ctx.tx.isFieldPresent(sfSigners))
-        {
-            // no signers, no signature - a valid simulation
-            return tesSUCCESS;
-        }
-
-        for (auto const& signer : ctx.tx.getFieldArray(sfSigners))
-        {
-            if (signer.isFieldPresent(sfTxnSignature) &&
-                !signer[sfTxnSignature].empty())
-            {
-                // NOTE: This code should never be hit because it's
-                // checked in the `simulate` RPC
-                return temINVALID;  // LCOV_EXCL_LINE
-            }
-        }
-
-        if (!ctx.tx.getSigningPubKey().empty())
-        {
-            // trying to single-sign _and_ multi-sign a transaction
-            return temINVALID;
-        }
-        return tesSUCCESS;
-    }
+    if (auto const ret =
+            detail::preflightCheckSimulateKeys(ctx.flags, ctx.tx, ctx.j))
+        // Skips following checks if the transaction is being simulated,
+        // regardless of success or failure
+        return *ret;
 
     auto const sigValid = checkValidity(
         ctx.app.getHashRouter(), ctx.tx, ctx.rules, ctx.app.config());
@@ -213,6 +251,28 @@ Transactor::Transactor(ApplyContext& ctx)
 {
 }
 
+bool
+Transactor::validDataLength(
+    std::optional<Slice> const& slice,
+    std::size_t maxLength)
+{
+    if (!slice)
+        return true;
+    return !slice->empty() && slice->length() <= maxLength;
+}
+
+std::uint32_t
+Transactor::getFlagsMask(PreflightContext const& ctx)
+{
+    return tfUniversalMask;
+}
+
+NotTEC
+Transactor::preflightSigValidated(PreflightContext const& ctx)
+{
+    return tesSUCCESS;
+}
+
 TER
 Transactor::checkPermission(ReadView const& view, STTx const& tx)
 {
@@ -247,6 +307,27 @@ Transactor::calculateBaseFee(ReadView const& view, STTx const& tx)
     return baseFee + (signerCount * baseFee);
 }
 
+// Returns the fee in fee units, not scaled for load.
+XRPAmount
+Transactor::calculateOwnerReserveFee(ReadView const& view, STTx const& tx)
+{
+    // Assumption: One reserve increment is typically much greater than one base
+    // fee.
+    // This check is in an assert so that it will come to the attention of
+    // developers if that assumption is not correct. If the owner reserve is not
+    // significantly larger than the base fee (or even worse, smaller), we will
+    // need to rethink charging an owner reserve as a transaction fee.
+    // TODO: This function is static, and I don't want to add more parameters.
+    // When it is finally refactored to be in a context that has access to the
+    // Application, include "app().overlay().networkID() > 2 ||" in the
+    // condition.
+    XRPL_ASSERT(
+        view.fees().increment > view.fees().base * 100,
+        "ripple::Transactor::calculateOwnerReserveFee : Owner reserve is "
+        "reasonable");
+    return view.fees().increment;
+}
+
 XRPAmount
 Transactor::minimumFee(
     Application& app,
@@ -578,16 +659,19 @@ Transactor::apply()
 }
 
 NotTEC
-Transactor::checkSign(PreclaimContext const& ctx)
+Transactor::checkSign(
+    PreclaimContext const& ctx,
+    AccountID const& idAccount,
+    STObject const& sigObject)
 {
-    auto const pkSigner = ctx.tx.getSigningPubKey();
+    auto const pkSigner = sigObject.getFieldVL(sfSigningPubKey);
     // Ignore signature check on batch inner transactions
-    if (ctx.tx.isFlag(tfInnerBatchTxn) &&
+    if (sigObject.isFlag(tfInnerBatchTxn) &&
         ctx.view.rules().enabled(featureBatch))
     {
         // Defensive Check: These values are also checked in Batch::preflight
-        if (ctx.tx.isFieldPresent(sfTxnSignature) || !pkSigner.empty() ||
-            ctx.tx.isFieldPresent(sfSigners))
+        if (sigObject.isFieldPresent(sfTxnSignature) || !pkSigner.empty() ||
+            sigObject.isFieldPresent(sfSigners))
         {
             return temINVALID_FLAG;  // LCOV_EXCL_LINE
         }
@@ -595,34 +679,31 @@ Transactor::checkSign(PreclaimContext const& ctx)
     }
 
     if ((ctx.flags & tapDRY_RUN) && pkSigner.empty() &&
-        !ctx.tx.isFieldPresent(sfSigners))
+        !sigObject.isFieldPresent(sfSigners))
     {
         // simulate: skip signature validation when neither SigningPubKey nor
         // Signers are provided
         return tesSUCCESS;
     }
 
-    auto const idAccount = ctx.tx[~sfDelegate].value_or(ctx.tx[sfAccount]);
-
     // If the pk is empty and not simulate or simulate and signers,
     // then we must be multi-signing.
     if (ctx.tx.isFieldPresent(sfSigners))
     {
-        STArray const& txSigners(ctx.tx.getFieldArray(sfSigners));
-        return checkMultiSign(ctx.view, idAccount, txSigners, ctx.flags, ctx.j);
+        return checkMultiSign(ctx, idAccount, sigObject);
     }
 
     // Check Single Sign
     XRPL_ASSERT(
-        !pkSigner.empty(),
-        "ripple::Transactor::checkSingleSign : non-empty signer or simulation");
+        !pkSigner.empty(), "ripple::Transactor::checkSign : non-empty signer");
 
     if (!publicKeyType(makeSlice(pkSigner)))
     {
-        JLOG(ctx.j.trace())
-            << "checkSingleSign: signing public key type is unknown";
+        JLOG(ctx.j.trace()) << "checkSign: signing public key type is unknown";
         return tefBAD_AUTH;  // FIXME: should be better error!
     }
+
+    // Look up the account.
     auto const idSigner = pkSigner.empty()
         ? idAccount
         : calcAccountID(PublicKey(makeSlice(pkSigner)));
@@ -630,8 +711,16 @@ Transactor::checkSign(PreclaimContext const& ctx)
     if (!sleAccount)
         return terNO_ACCOUNT;
 
-    return checkSingleSign(
-        idSigner, idAccount, sleAccount, ctx.view.rules(), ctx.j);
+    return checkSingleSign(ctx, idSigner, idAccount, sleAccount);
+}
+
+NotTEC
+Transactor::checkSign(PreclaimContext const& ctx)
+{
+    auto const idAccount = ctx.tx.isFieldPresent(sfDelegate)
+        ? ctx.tx.getAccountID(sfDelegate)
+        : ctx.tx.getAccountID(sfAccount);
+    return checkSign(ctx, idAccount, ctx.tx);
 }
 
 NotTEC
@@ -646,9 +735,7 @@ Transactor::checkBatchSign(PreclaimContext const& ctx)
         Blob const& pkSigner = signer.getFieldVL(sfSigningPubKey);
         if (pkSigner.empty())
         {
-            STArray const& txSigners(signer.getFieldArray(sfSigners));
-            if (ret = checkMultiSign(
-                    ctx.view, idAccount, txSigners, ctx.flags, ctx.j);
+            if (ret = checkMultiSign(ctx, idAccount, signer);
                 !isTesSuccess(ret))
                 return ret;
         }
@@ -672,8 +759,7 @@ Transactor::checkBatchSign(PreclaimContext const& ctx)
                 return tesSUCCESS;
             }
 
-            if (ret = checkSingleSign(
-                    idSigner, idAccount, sleAccount, ctx.view.rules(), ctx.j);
+            if (ret = checkSingleSign(ctx, idSigner, idAccount, sleAccount);
                 !isTesSuccess(ret))
                 return ret;
         }
@@ -683,15 +769,14 @@ Transactor::checkBatchSign(PreclaimContext const& ctx)
 
 NotTEC
 Transactor::checkSingleSign(
+    PreclaimContext const& ctx,
     AccountID const& idSigner,
     AccountID const& idAccount,
-    std::shared_ptr<SLE const> sleAccount,
-    Rules const& rules,
-    beast::Journal j)
+    std::shared_ptr<SLE const> sleAccount)
 {
     bool const isMasterDisabled = sleAccount->isFlag(lsfDisableMaster);
 
-    if (rules.enabled(fixMasterKeyAsRegularKey))
+    if (ctx.view.rules().enabled(fixMasterKeyAsRegularKey))
     {
         // Signed with regular key.
         if ((*sleAccount)[~sfRegularKey] == idSigner)
@@ -728,14 +813,16 @@ Transactor::checkSingleSign(
     else if (sleAccount->isFieldPresent(sfRegularKey))
     {
         // Signing key does not match master or regular key.
-        JLOG(j.trace()) << "checkSingleSign: Not authorized to use account.";
+        JLOG(ctx.j.trace())
+            << "checkSingleSign: Not authorized to use account.";
         return tefBAD_AUTH;
     }
     else
     {
         // No regular key on account and signing key does not match master key.
         // FIXME: Why differentiate this case from tefBAD_AUTH?
-        JLOG(j.trace()) << "checkSingleSign: Not authorized to use account.";
+        JLOG(ctx.j.trace())
+            << "checkSingleSign: Not authorized to use account.";
         return tefBAD_AUTH_MASTER;
     }
 
@@ -744,19 +831,17 @@ Transactor::checkSingleSign(
 
 NotTEC
 Transactor::checkMultiSign(
-    ReadView const& view,
+    PreclaimContext const& ctx,
     AccountID const& id,
-    STArray const& txSigners,
-    ApplyFlags const& flags,
-    beast::Journal j)
+    STObject const& sigObject)
 {
-    // Get mTxnAccountID's SignerList and Quorum.
+    // Get id's SignerList and Quorum.
     std::shared_ptr<STLedgerEntry const> sleAccountSigners =
-        view.read(keylet::signers(id));
+        ctx.view.read(keylet::signers(id));
     // If the signer list doesn't exist the account is not multi-signing.
     if (!sleAccountSigners)
     {
-        JLOG(j.trace())
+        JLOG(ctx.j.trace())
             << "applyTransaction: Invalid: Not a multi-signing account.";
         return tefNOT_MULTI_SIGNING;
     }
@@ -771,11 +856,12 @@ Transactor::checkMultiSign(
         "ripple::Transactor::checkMultiSign : signer list ID is 0");
 
     auto accountSigners =
-        SignerEntries::deserialize(*sleAccountSigners, j, "ledger");
+        SignerEntries::deserialize(*sleAccountSigners, ctx.j, "ledger");
     if (!accountSigners)
         return accountSigners.error();
 
     // Get the array of transaction signers.
+    STArray const& txSigners(sigObject.getFieldArray(sfSigners));
 
     // Walk the accountSigners performing a variety of checks and see if
     // the quorum is met.
@@ -794,7 +880,7 @@ Transactor::checkMultiSign(
         {
             if (++iter == accountSigners->end())
             {
-                JLOG(j.trace())
+                JLOG(ctx.j.trace())
                     << "applyTransaction: Invalid SigningAccount.Account.";
                 return tefBAD_SIGNATURE;
             }
@@ -802,7 +888,7 @@ Transactor::checkMultiSign(
         if (iter->account != txSignerAcctID)
         {
             // The SigningAccount is not in the SignerEntries.
-            JLOG(j.trace())
+            JLOG(ctx.j.trace())
                 << "applyTransaction: Invalid SigningAccount.Account.";
             return tefBAD_SIGNATURE;
         }
@@ -816,13 +902,13 @@ Transactor::checkMultiSign(
         // STTx::checkMultiSign
         if (!spk.empty() && !publicKeyType(makeSlice(spk)))
         {
-            JLOG(j.trace())
+            JLOG(ctx.j.trace())
                 << "checkMultiSign: signing public key type is unknown";
             return tefBAD_SIGNATURE;
         }
 
         XRPL_ASSERT(
-            (flags & tapDRY_RUN) || !spk.empty(),
+            (ctx.flags & tapDRY_RUN) || !spk.empty(),
             "ripple::Transactor::checkMultiSign : non-empty signer or "
             "simulation");
         AccountID const signingAcctIDFromPubKey = spk.empty()
@@ -854,7 +940,8 @@ Transactor::checkMultiSign(
 
         // In any of these cases we need to know whether the account is in
         // the ledger.  Determine that now.
-        auto const sleTxSignerRoot = view.read(keylet::account(txSignerAcctID));
+        auto const sleTxSignerRoot =
+            ctx.view.read(keylet::account(txSignerAcctID));
 
         if (signingAcctIDFromPubKey == txSignerAcctID)
         {
@@ -867,7 +954,7 @@ Transactor::checkMultiSign(
 
                 if (signerAccountFlags & lsfDisableMaster)
                 {
-                    JLOG(j.trace())
+                    JLOG(ctx.j.trace())
                         << "applyTransaction: Signer:Account lsfDisableMaster.";
                     return tefMASTER_DISABLED;
                 }
@@ -879,21 +966,21 @@ Transactor::checkMultiSign(
             // Public key must hash to the account's regular key.
             if (!sleTxSignerRoot)
             {
-                JLOG(j.trace()) << "applyTransaction: Non-phantom signer "
-                                   "lacks account root.";
+                JLOG(ctx.j.trace()) << "applyTransaction: Non-phantom signer "
+                                       "lacks account root.";
                 return tefBAD_SIGNATURE;
             }
 
             if (!sleTxSignerRoot->isFieldPresent(sfRegularKey))
             {
-                JLOG(j.trace())
+                JLOG(ctx.j.trace())
                     << "applyTransaction: Account lacks RegularKey.";
                 return tefBAD_SIGNATURE;
             }
             if (signingAcctIDFromPubKey !=
                 sleTxSignerRoot->getAccountID(sfRegularKey))
             {
-                JLOG(j.trace())
+                JLOG(ctx.j.trace())
                     << "applyTransaction: Account doesn't match RegularKey.";
                 return tefBAD_SIGNATURE;
             }
@@ -905,7 +992,8 @@ Transactor::checkMultiSign(
     // Cannot perform transaction if quorum is not met.
     if (weightSum < sleAccountSigners->getFieldU32(sfSignerQuorum))
     {
-        JLOG(j.trace()) << "applyTransaction: Signers failed to meet quorum.";
+        JLOG(ctx.j.trace())
+            << "applyTransaction: Signers failed to meet quorum.";
         return tefBAD_QUORUM;
     }
 

src/xrpld/app/tx/detail/Transactor.h

@@ -134,6 +134,8 @@ public:
 
 class TxConsequences;
 struct PreflightResult;
+// Needed for preflight specialization
+class Change;
 
 class Transactor
 {
@@ -198,6 +200,35 @@ public:
     static XRPAmount
     calculateBaseFee(ReadView const& view, STTx const& tx);
 
+    /* Do NOT define an invokePreflight function in a derived class.
+       Instead, define:
+
+        // Optional if the transaction is gated on an amendment that
+        // isn't specified in transactions.macro
+        static bool
+        checkExtraFeatures(PreflightContext const& ctx);
+
+        // Optional if the transaction uses any flags other than tfUniversal
+        static std::uint32_t
+        getFlagsMask(PreflightContext const& ctx);
+
+        // Required, even if it just returns tesSUCCESS.
+        static NotTEC
+        preflight(PreflightContext const& ctx);
+
+        // Optional, rarely needed, if the transaction does any expensive
+        // checks after the signature is verified.
+        static NotTEC preflightSigValidated(PreflightContext const& ctx);
+
+       * Do not try to call preflight1 or preflight2 directly.
+       * Do not check whether relevant amendments are enabled in preflight.
+         Instead, define checkExtraFeatures.
+       * Do not check flags in preflight. Instead, define getFlagsMask.
+    */
+    template <class T>
+    static NotTEC
+    invokePreflight(PreflightContext const& ctx);
+
     static TER
     preclaim(PreclaimContext const& ctx)
     {
@@ -246,6 +277,42 @@ protected:
         Fees const& fees,
         ApplyFlags flags);
 
+    // Returns the fee in fee units, not scaled for load.
+    static XRPAmount
+    calculateOwnerReserveFee(ReadView const& view, STTx const& tx);
+
+    static NotTEC
+    checkSign(
+        PreclaimContext const& ctx,
+        AccountID const& id,
+        STObject const& sigObject);
+
+    // Base class always returns true
+    static bool
+    checkExtraFeatures(PreflightContext const& ctx);
+
+    // Base class always returns tfUniversalMask
+    static std::uint32_t
+    getFlagsMask(PreflightContext const& ctx);
+
+    // Base class always returns tesSUCCESS
+    static NotTEC
+    preflightSigValidated(PreflightContext const& ctx);
+
+    static bool
+    validDataLength(std::optional<Slice> const& slice, std::size_t maxLength);
+
+    template <class T>
+    static bool
+    validNumericRange(std::optional<T> value, T max, T min = {});
+
+    template <class T, class Unit>
+    static bool
+    validNumericRange(
+        std::optional<T> value,
+        unit::ValueUnit<Unit, T> max,
+        unit::ValueUnit<Unit, T> min = {});
+
 private:
     std::pair<TER, XRPAmount>
     reset(XRPAmount fee);
@@ -256,33 +323,117 @@ private:
     payFee();
     static NotTEC
     checkSingleSign(
+        PreclaimContext const& ctx,
         AccountID const& idSigner,
         AccountID const& idAccount,
-        std::shared_ptr<SLE const> sleAccount,
-        Rules const& rules,
-        beast::Journal j);
+        std::shared_ptr<SLE const> sleAccount);
     static NotTEC
     checkMultiSign(
-        ReadView const& view,
-        AccountID const& idAccount,
-        STArray const& txSigners,
-        ApplyFlags const& flags,
-        beast::Journal j);
+        PreclaimContext const& ctx,
+        AccountID const& id,
+        STObject const& sigObject);
 
     void trapTransaction(uint256) const;
+
+    /** Performs early sanity checks on the account and fee fields.
+
+        (And passes flagMask to preflight0)
+
+        Do not try to call preflight1 from preflight() in derived classes. See
+        the description of invokePreflight for details.
+    */
+    static NotTEC
+    preflight1(PreflightContext const& ctx, std::uint32_t flagMask);
+
+    /** Checks whether the signature appears valid
+
+        Do not try to call preflight2 from preflight() in derived classes. See
+        the description of invokePreflight for details.
+    */
+    static NotTEC
+    preflight2(PreflightContext const& ctx);
 };
 
-/** Performs early sanity checks on the txid */
-NotTEC
-preflight0(PreflightContext const& ctx);
+inline bool
+Transactor::checkExtraFeatures(PreflightContext const& ctx)
+{
+    return true;
+}
 
-/** Performs early sanity checks on the account and fee fields */
+/** Performs early sanity checks on the txid and flags */
 NotTEC
-preflight1(PreflightContext const& ctx);
+preflight0(PreflightContext const& ctx, std::uint32_t flagMask);
 
-/** Checks whether the signature appears valid */
+namespace detail {
+
+/** Checks the validity of the transactor signing key.
+ *
+ * Normally called from preflight1 with ctx.tx.
+ */
 NotTEC
-preflight2(PreflightContext const& ctx);
+preflightCheckSigningKey(STObject const& sigObject, beast::Journal j);
+
+/** Checks the special signing key state needed for simulation
+ *
+ * Normally called from preflight2 with ctx.tx.
+ */
+std::optional<NotTEC>
+preflightCheckSimulateKeys(
+    ApplyFlags flags,
+    STObject const& sigObject,
+    beast::Journal j);
+}  // namespace detail
+
+// Defined in Change.cpp
+template <>
+NotTEC
+Transactor::invokePreflight<Change>(PreflightContext const& ctx);
+
+template <class T>
+NotTEC
+Transactor::invokePreflight(PreflightContext const& ctx)
+{
+    // Using this lookup does NOT require checking the fixDelegateV1_1. The data
+    // exists regardless of whether it is enabled.
+    auto const feature =
+        Permission::getInstance().getTxFeature(ctx.tx.getTxnType());
+
+    if (feature && !ctx.rules.enabled(*feature))
+        return temDISABLED;
+
+    if (!T::checkExtraFeatures(ctx))
+        return temDISABLED;
+
+    if (auto const ret = preflight1(ctx, T::getFlagsMask(ctx)))
+        return ret;
+
+    if (auto const ret = T::preflight(ctx))
+        return ret;
+
+    if (auto const ret = preflight2(ctx))
+        return ret;
+
+    return T::preflightSigValidated(ctx);
+}
+
+template <class T>
+bool
+Transactor::validNumericRange(std::optional<T> value, T max, T min)
+{
+    if (!value)
+        return true;
+    return value >= min && value <= max;
+}
+
+template <class T, class Unit>
+bool
+Transactor::validNumericRange(
+    std::optional<T> value,
+    unit::ValueUnit<Unit, T> max,
+    unit::ValueUnit<Unit, T> min)
+{
+    return validNumericRange(value, max.value(), min.value());
+}
 
 }  // namespace ripple
 

src/xrpld/app/tx/detail/VaultClawback.cpp

@@ -35,15 +35,6 @@ namespace ripple {
 NotTEC
 VaultClawback::preflight(PreflightContext const& ctx)
 {
-    if (!ctx.rules.enabled(featureSingleAssetVault))
-        return temDISABLED;
-
-    if (auto const ter = preflight1(ctx))
-        return ter;
-
-    if (ctx.tx.getFlags() & tfUniversalMask)
-        return temINVALID_FLAG;
-
     if (ctx.tx[sfVaultID] == beast::zero)
     {
         JLOG(ctx.j.debug()) << "VaultClawback: zero/empty vault ID.";
@@ -78,7 +69,7 @@ VaultClawback::preflight(PreflightContext const& ctx)
         }
     }
 
-    return preflight2(ctx);
+    return tesSUCCESS;
 }
 
 TER

src/xrpld/app/tx/detail/VaultCreate.cpp

@@ -35,28 +35,27 @@
 
 namespace ripple {
 
+bool
+VaultCreate::checkExtraFeatures(PreflightContext const& ctx)
+{
+    if (!ctx.rules.enabled(featureMPTokensV1))
+        return false;
+
+    return !ctx.tx.isFieldPresent(sfDomainID) ||
+        ctx.rules.enabled(featurePermissionedDomains);
+}
+
+std::uint32_t
+VaultCreate::getFlagsMask(PreflightContext const& ctx)
+{
+    return tfVaultCreateMask;
+}
+
 NotTEC
 VaultCreate::preflight(PreflightContext const& ctx)
 {
-    if (!ctx.rules.enabled(featureSingleAssetVault) ||
-        !ctx.rules.enabled(featureMPTokensV1))
-        return temDISABLED;
-
-    if (ctx.tx.isFieldPresent(sfDomainID) &&
-        !ctx.rules.enabled(featurePermissionedDomains))
-        return temDISABLED;
-
-    if (auto const ter = preflight1(ctx))
-        return ter;
-
-    if (ctx.tx.getFlags() & tfVaultCreateMask)
-        return temINVALID_FLAG;
-
-    if (auto const data = ctx.tx[~sfData])
-    {
-        if (data->empty() || data->length() > maxDataPayloadLength)
-            return temMALFORMED;
-    }
+    if (!validDataLength(ctx.tx[~sfData], maxDataPayloadLength))
+        return temMALFORMED;
 
     if (auto const withdrawalPolicy = ctx.tx[~sfWithdrawalPolicy])
     {
@@ -96,14 +95,14 @@ VaultCreate::preflight(PreflightContext const& ctx)
             return temMALFORMED;
     }
 
-    return preflight2(ctx);
+    return tesSUCCESS;
 }
 
 XRPAmount
 VaultCreate::calculateBaseFee(ReadView const& view, STTx const& tx)
 {
     // One reserve increment is typically much greater than one base fee.
-    return view.fees().increment;
+    return calculateOwnerReserveFee(view, tx);
 }
 
 TER
@@ -112,32 +111,8 @@ VaultCreate::preclaim(PreclaimContext const& ctx)
     auto const vaultAsset = ctx.tx[sfAsset];
     auto const account = ctx.tx[sfAccount];
 
-    if (vaultAsset.native())
-        ;  // No special checks for XRP
-    else if (vaultAsset.holds<MPTIssue>())
-    {
-        auto mptID = vaultAsset.get<MPTIssue>().getMptID();
-        auto issuance = ctx.view.read(keylet::mptIssuance(mptID));
-        if (!issuance)
-            return tecOBJECT_NOT_FOUND;
-        if (!issuance->isFlag(lsfMPTCanTransfer))
-        {
-            // NOTE: flag lsfMPTCanTransfer is immutable, so this is debug in
-            // VaultCreate only; in other vault function it's an error.
-            JLOG(ctx.j.debug())
-                << "VaultCreate: vault assets are non-transferable.";
-            return tecNO_AUTH;
-        }
-    }
-    else if (vaultAsset.holds<Issue>())
-    {
-        auto const issuer =
-            ctx.view.read(keylet::account(vaultAsset.getIssuer()));
-        if (!issuer)
-            return terNO_ACCOUNT;
-        else if (!issuer->isFlag(lsfDefaultRipple))
-            return terNO_RIPPLE;
-    }
+    if (auto const ter = canAddHolding(ctx.view, vaultAsset))
+        return ter;
 
     // Check for pseudo-account issuers - we do not want a vault to hold such
     // assets (e.g. MPT shares to other vaults or AMM LPTokens) as they would be

src/xrpld/app/tx/detail/VaultCreate.h

@@ -33,6 +33,12 @@ public:
     {
     }
 
+    static bool
+    checkExtraFeatures(PreflightContext const& ctx);
+
+    static std::uint32_t
+    getFlagsMask(PreflightContext const& ctx);
+
     static NotTEC
     preflight(PreflightContext const& ctx);
 

src/xrpld/app/tx/detail/VaultDelete.cpp

@@ -31,22 +31,13 @@ namespace ripple {
 NotTEC
 VaultDelete::preflight(PreflightContext const& ctx)
 {
-    if (!ctx.rules.enabled(featureSingleAssetVault))
-        return temDISABLED;
-
-    if (auto const ter = preflight1(ctx))
-        return ter;
-
-    if (ctx.tx.getFlags() & tfUniversalMask)
-        return temINVALID_FLAG;
-
     if (ctx.tx[sfVaultID] == beast::zero)
     {
         JLOG(ctx.j.debug()) << "VaultDelete: zero/empty vault ID.";
         return temMALFORMED;
     }
 
-    return preflight2(ctx);
+    return tesSUCCESS;
 }
 
 TER

src/xrpld/app/tx/detail/VaultDeposit.cpp

@@ -36,15 +36,6 @@ namespace ripple {
 NotTEC
 VaultDeposit::preflight(PreflightContext const& ctx)
 {
-    if (!ctx.rules.enabled(featureSingleAssetVault))
-        return temDISABLED;
-
-    if (auto const ter = preflight1(ctx))
-        return ter;
-
-    if (ctx.tx.getFlags() & tfUniversalMask)
-        return temINVALID_FLAG;
-
     if (ctx.tx[sfVaultID] == beast::zero)
     {
         JLOG(ctx.j.debug()) << "VaultDeposit: zero/empty vault ID.";
@@ -54,7 +45,7 @@ VaultDeposit::preflight(PreflightContext const& ctx)
     if (ctx.tx[sfAmount] <= beast::zero)
         return temBAD_AMOUNT;
 
-    return preflight2(ctx);
+    return tesSUCCESS;
 }
 
 TER