From 40198d9792af68df4f5a7610c614e8420c487d7b Mon Sep 17 00:00:00 2001 From: Bart Date: Mon, 22 Dec 2025 16:30:23 -0500 Subject: [PATCH 1/2] ci: Remove superfluous build directory creation (#6159) This change modifies the build directory structure from `build/build/xxx` or `.build/build/xxx` to just `build/xxx`. Namely, the `conanfile.py` has the CMake generators build directory hardcoded to `build/generators`. We may as well leverage the top-level build directory without introducing another layer of directory nesting. --- .github/actions/build-deps/action.yml | 9 +----- .github/workflows/publish-docs.yml | 2 +- .../workflows/reusable-build-test-config.yml | 29 +++++++++---------- .github/workflows/reusable-build-test.yml | 6 ---- .github/workflows/upload-conan-deps.yml | 1 - 5 files changed, 15 insertions(+), 32 deletions(-) diff --git a/.github/actions/build-deps/action.yml b/.github/actions/build-deps/action.yml index f20eb3a595..d1fb980dac 100644 --- a/.github/actions/build-deps/action.yml +++ b/.github/actions/build-deps/action.yml @@ -4,9 +4,6 @@ description: "Install Conan dependencies, optionally forcing a rebuild of all de # 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_type: description: 'The build type to use ("Debug", "Release").' required: true @@ -28,17 +25,13 @@ runs: - name: Install Conan dependencies shell: bash env: - BUILD_DIR: ${{ inputs.build_dir }} BUILD_NPROC: ${{ inputs.build_nproc }} BUILD_OPTION: ${{ inputs.force_build == 'true' && '*' || 'missing' }} BUILD_TYPE: ${{ inputs.build_type }} LOG_VERBOSITY: ${{ inputs.log_verbosity }} run: | echo 'Installing dependencies.' - mkdir -p "${BUILD_DIR}" - cd "${BUILD_DIR}" conan install \ - --output-folder . \ --build="${BUILD_OPTION}" \ --options:host='&:tests=True' \ --options:host='&:xrpld=True' \ @@ -46,4 +39,4 @@ runs: --conf:all tools.build:jobs=${BUILD_NPROC} \ --conf:all tools.build:verbosity="${LOG_VERBOSITY}" \ --conf:all tools.compilation:verbosity="${LOG_VERBOSITY}" \ - .. + . diff --git a/.github/workflows/publish-docs.yml b/.github/workflows/publish-docs.yml index 6f0927a2b3..c37a82a2f3 100644 --- a/.github/workflows/publish-docs.yml +++ b/.github/workflows/publish-docs.yml @@ -22,7 +22,7 @@ defaults: shell: bash env: - BUILD_DIR: .build + BUILD_DIR: build NPROC_SUBTRACT: 2 jobs: diff --git a/.github/workflows/reusable-build-test-config.yml b/.github/workflows/reusable-build-test-config.yml index 43acfab542..98bf107225 100644 --- a/.github/workflows/reusable-build-test-config.yml +++ b/.github/workflows/reusable-build-test-config.yml @@ -3,11 +3,6 @@ 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 @@ -59,6 +54,11 @@ defaults: run: shell: bash +env: + # Conan installs the generators in the build/generators directory, see the + # layout() method in conanfile.py. We then run CMake from the build directory. + BUILD_DIR: build + jobs: build-and-test: name: ${{ inputs.config_name }} @@ -96,7 +96,6 @@ jobs: - name: Build dependencies uses: ./.github/actions/build-deps with: - build_dir: ${{ inputs.build_dir }} build_nproc: ${{ steps.nproc.outputs.nproc }} build_type: ${{ inputs.build_type }} # Set the verbosity to "quiet" for Windows to avoid an excessive @@ -104,7 +103,7 @@ jobs: log_verbosity: ${{ runner.os == 'Windows' && 'quiet' || 'verbose' }} - name: Configure CMake - working-directory: ${{ inputs.build_dir }} + working-directory: ${{ env.BUILD_DIR }} env: BUILD_TYPE: ${{ inputs.build_type }} CMAKE_ARGS: ${{ inputs.cmake_args }} @@ -117,7 +116,7 @@ jobs: .. - name: Build the binary - working-directory: ${{ inputs.build_dir }} + working-directory: ${{ env.BUILD_DIR }} env: BUILD_NPROC: ${{ steps.nproc.outputs.nproc }} BUILD_TYPE: ${{ inputs.build_type }} @@ -132,8 +131,6 @@ jobs: - name: Upload the binary (Linux) if: ${{ github.repository_owner == 'XRPLF' && runner.os == 'Linux' }} uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2 - env: - BUILD_DIR: ${{ inputs.build_dir }} with: name: xrpld-${{ inputs.config_name }} path: ${{ env.BUILD_DIR }}/xrpld @@ -142,7 +139,7 @@ jobs: - name: Check linking (Linux) if: ${{ runner.os == 'Linux' }} - working-directory: ${{ inputs.build_dir }} + working-directory: ${{ env.BUILD_DIR }} run: | ldd ./xrpld if [ "$(ldd ./xrpld | grep -E '(libstdc\+\+|libgcc)' | wc -l)" -eq 0 ]; then @@ -154,13 +151,13 @@ jobs: - name: Verify presence of instrumentation (Linux) if: ${{ runner.os == 'Linux' && env.ENABLED_VOIDSTAR == 'true' }} - working-directory: ${{ inputs.build_dir }} + working-directory: ${{ env.BUILD_DIR }} run: | ./xrpld --version | grep libvoidstar - name: Run the separate tests if: ${{ !inputs.build_only }} - working-directory: ${{ inputs.build_dir }} + working-directory: ${{ env.BUILD_DIR }} # Windows locks some of the build files while running tests, and parallel jobs can collide env: BUILD_TYPE: ${{ inputs.build_type }} @@ -173,7 +170,7 @@ jobs: - name: Run the embedded tests if: ${{ !inputs.build_only }} - working-directory: ${{ runner.os == 'Windows' && format('{0}/{1}', inputs.build_dir, inputs.build_type) || inputs.build_dir }} + working-directory: ${{ runner.os == 'Windows' && format('{0}/{1}', env.BUILD_DIR, inputs.build_type) || env.BUILD_DIR }} env: BUILD_NPROC: ${{ steps.nproc.outputs.nproc }} run: | @@ -189,7 +186,7 @@ jobs: - name: Prepare coverage report if: ${{ !inputs.build_only && env.ENABLED_COVERAGE == 'true' }} - working-directory: ${{ inputs.build_dir }} + working-directory: ${{ env.BUILD_DIR }} env: BUILD_NPROC: ${{ steps.nproc.outputs.nproc }} BUILD_TYPE: ${{ inputs.build_type }} @@ -207,7 +204,7 @@ jobs: disable_search: true disable_telem: true fail_ci_if_error: true - files: ${{ inputs.build_dir }}/coverage.xml + files: ${{ env.BUILD_DIR }}/coverage.xml plugins: noop token: ${{ secrets.CODECOV_TOKEN }} verbose: true diff --git a/.github/workflows/reusable-build-test.yml b/.github/workflows/reusable-build-test.yml index c6e991df79..7f14aacb9b 100644 --- a/.github/workflows/reusable-build-test.yml +++ b/.github/workflows/reusable-build-test.yml @@ -8,11 +8,6 @@ name: Build and test on: workflow_call: inputs: - build_dir: - description: "The directory where to build." - required: false - type: string - default: ".build" os: description: 'The operating system to use for the build ("linux", "macos", "windows").' required: true @@ -46,7 +41,6 @@ jobs: matrix: ${{ fromJson(needs.generate-matrix.outputs.matrix) }} max-parallel: 10 with: - build_dir: ${{ inputs.build_dir }} build_only: ${{ matrix.build_only }} build_type: ${{ matrix.build_type }} cmake_args: ${{ matrix.cmake_args }} diff --git a/.github/workflows/upload-conan-deps.yml b/.github/workflows/upload-conan-deps.yml index ec283e564c..5024666394 100644 --- a/.github/workflows/upload-conan-deps.yml +++ b/.github/workflows/upload-conan-deps.yml @@ -92,7 +92,6 @@ jobs: - name: Build dependencies uses: ./.github/actions/build-deps with: - build_dir: .build build_nproc: ${{ steps.nproc.outputs.nproc }} build_type: ${{ matrix.build_type }} force_build: ${{ github.event_name == 'schedule' || github.event.inputs.force_source_build == 'true' }} From c81b26b09e2c29a384d0129573c03df5b597ed89 Mon Sep 17 00:00:00 2001 From: Ed Hennis Date: Mon, 22 Dec 2025 17:33:44 -0500 Subject: [PATCH 2/2] Add a few minor changes (#6158) - Updates or fixes a couple of things I noticed while reviewing changes to the spec. - Rename sfPreviousPaymentDate to sfPreviousPaymentDueDate. - Make the vault asset cap check added in #6124 a little more robust: 1. Check in preflight if the vault is _already_ over the limit. 2. Prevent overflow when checking with the loan value. (Subtract instead of adding, in case the values are near maxint. Both return the same result. Also add a unit test so each case is covered. --- .../xrpl/protocol/detail/ledger_entries.macro | 2 +- include/xrpl/protocol/detail/sfields.macro | 2 +- src/test/app/Loan_test.cpp | 40 +++++++++++++++---- src/xrpld/app/misc/detail/LendingHelpers.cpp | 2 +- src/xrpld/app/tx/detail/LoanManage.cpp | 3 +- src/xrpld/app/tx/detail/LoanSet.cpp | 19 +++++++-- 6 files changed, 54 insertions(+), 14 deletions(-) diff --git a/include/xrpl/protocol/detail/ledger_entries.macro b/include/xrpl/protocol/detail/ledger_entries.macro index 1034c35895..109a62b6ac 100644 --- a/include/xrpl/protocol/detail/ledger_entries.macro +++ b/include/xrpl/protocol/detail/ledger_entries.macro @@ -541,7 +541,7 @@ LEDGER_ENTRY(ltLOAN, 0x0089, Loan, loan, ({ {sfStartDate, soeREQUIRED}, {sfPaymentInterval, soeREQUIRED}, {sfGracePeriod, soeDEFAULT}, - {sfPreviousPaymentDate, soeDEFAULT}, + {sfPreviousPaymentDueDate, soeDEFAULT}, {sfNextPaymentDueDate, soeDEFAULT}, // The loan object tracks these values: // diff --git a/include/xrpl/protocol/detail/sfields.macro b/include/xrpl/protocol/detail/sfields.macro index d5c5d9447f..d0736469e4 100644 --- a/include/xrpl/protocol/detail/sfields.macro +++ b/include/xrpl/protocol/detail/sfields.macro @@ -102,7 +102,7 @@ TYPED_SFIELD(sfMutableFlags, UINT32, 53) TYPED_SFIELD(sfStartDate, UINT32, 54) TYPED_SFIELD(sfPaymentInterval, UINT32, 55) TYPED_SFIELD(sfGracePeriod, UINT32, 56) -TYPED_SFIELD(sfPreviousPaymentDate, UINT32, 57) +TYPED_SFIELD(sfPreviousPaymentDueDate, UINT32, 57) TYPED_SFIELD(sfNextPaymentDueDate, UINT32, 58) TYPED_SFIELD(sfPaymentRemaining, UINT32, 59) TYPED_SFIELD(sfPaymentTotal, UINT32, 60) diff --git a/src/test/app/Loan_test.cpp b/src/test/app/Loan_test.cpp index 9a92aed195..99863b484f 100644 --- a/src/test/app/Loan_test.cpp +++ b/src/test/app/Loan_test.cpp @@ -372,7 +372,7 @@ protected: if (auto loan = env.le(loanKeylet); env.test.BEAST_EXPECT(loan)) { env.test.BEAST_EXPECT( - loan->at(sfPreviousPaymentDate) == previousPaymentDate); + loan->at(sfPreviousPaymentDueDate) == previousPaymentDate); env.test.BEAST_EXPECT( loan->at(sfPaymentRemaining) == paymentRemaining); env.test.BEAST_EXPECT( @@ -507,7 +507,7 @@ protected: if (auto loan = env.le(loanKeylet); BEAST_EXPECT(loan)) { return LoanState{ - .previousPaymentDate = loan->at(sfPreviousPaymentDate), + .previousPaymentDate = loan->at(sfPreviousPaymentDueDate), .startDate = tp{d{loan->at(sfStartDate)}}, .nextPaymentDate = loan->at(sfNextPaymentDueDate), .paymentRemaining = loan->at(sfPaymentRemaining), @@ -1454,7 +1454,7 @@ protected: BEAST_EXPECT( loan->at(sfPaymentInterval) == *loanParams.payInterval); BEAST_EXPECT(loan->at(sfGracePeriod) == *loanParams.gracePd); - BEAST_EXPECT(loan->at(sfPreviousPaymentDate) == 0); + BEAST_EXPECT(loan->at(sfPreviousPaymentDueDate) == 0); BEAST_EXPECT( loan->at(sfNextPaymentDueDate) == startDate + *loanParams.payInterval); @@ -3597,13 +3597,39 @@ protected: env(tx); env.close(); - testcase("Vault maximum value exceeded"); + testcase("Vault at maximum value"); env(set(issuer, broker.brokerID, principalRequest), counterparty(lender), interestRate(TenthBips32(10'000)), sig(sfCounterpartySignature, lender), fee(env.current()->fees().base * 5), - ter(tecLIMIT_EXCEEDED)); + ter(tecLIMIT_EXCEEDED), + THISLINE); + }, + nullptr); + + testCase( + [&, this](Env& env, BrokerInfo const& broker, auto&) { + using namespace loan; + Number const principalRequest = broker.asset(1'000).value(); + Vault vault{env}; + auto tx = vault.set({.owner = lender, .id = broker.vaultID}); + tx[sfAssetsMaximum] = + BrokerParameters::defaults().vaultDeposit + + broker.asset(1).number(); + env(tx); + env.close(); + + testcase("Vault maximum value exceeded"); + env(set(issuer, broker.brokerID, principalRequest), + counterparty(lender), + interestRate(TenthBips32(100'000)), + sig(sfCounterpartySignature, lender), + fee(env.current()->fees().base * 5), + paymentTotal(2), + paymentInterval(3600 * 24), + ter(tecLIMIT_EXCEEDED), + THISLINE); }, nullptr); } @@ -3841,7 +3867,7 @@ protected: BEAST_EXPECT(loan[sfPaymentInterval] == 60); BEAST_EXPECT(loan[sfPeriodicPayment] == "1000000000"); BEAST_EXPECT(loan[sfPaymentRemaining] == 1); - BEAST_EXPECT(!loan.isMember(sfPreviousPaymentDate)); + BEAST_EXPECT(!loan.isMember(sfPreviousPaymentDueDate)); BEAST_EXPECT(loan[sfPrincipalOutstanding] == "1000000000"); BEAST_EXPECT(loan[sfTotalValueOutstanding] == "1000000000"); BEAST_EXPECT(!loan.isMember(sfLoanScale)); @@ -6048,7 +6074,7 @@ protected: { // --- PoC Summary ---------------------------------------------------- // Scenario: Borrower makes one periodic payment early (before next due) - // so doPayment sets sfPreviousPaymentDate to the (future) + // so doPayment sets sfPreviousPaymentDueDate to the (future) // sfNextPaymentDueDate and advances sfNextPaymentDueDate by one // interval. Borrower then immediately performs a full-payment // (tfLoanFullPayment). Why it matters: Full-payment interest accrual diff --git a/src/xrpld/app/misc/detail/LendingHelpers.cpp b/src/xrpld/app/misc/detail/LendingHelpers.cpp index dee53e2037..1dd47a7973 100644 --- a/src/xrpld/app/misc/detail/LendingHelpers.cpp +++ b/src/xrpld/app/misc/detail/LendingHelpers.cpp @@ -1669,7 +1669,7 @@ loanMakePayment( Number const periodicPayment = loan->at(sfPeriodicPayment); - auto prevPaymentDateProxy = loan->at(sfPreviousPaymentDate); + auto prevPaymentDateProxy = loan->at(sfPreviousPaymentDueDate); std::uint32_t const startDate = loan->at(sfStartDate); std::uint32_t const paymentInterval = loan->at(sfPaymentInterval); diff --git a/src/xrpld/app/tx/detail/LoanManage.cpp b/src/xrpld/app/tx/detail/LoanManage.cpp index f0cc2d9056..a09dbb2273 100644 --- a/src/xrpld/app/tx/detail/LoanManage.cpp +++ b/src/xrpld/app/tx/detail/LoanManage.cpp @@ -374,7 +374,8 @@ LoanManage::unimpairLoan( loanSle->clearFlag(lsfLoanImpaired); auto const paymentInterval = loanSle->at(sfPaymentInterval); auto const normalPaymentDueDate = - std::max(loanSle->at(sfPreviousPaymentDate), loanSle->at(sfStartDate)) + + std::max( + loanSle->at(sfPreviousPaymentDueDate), loanSle->at(sfStartDate)) + paymentInterval; if (!hasExpired(view, normalPaymentDueDate)) { diff --git a/src/xrpld/app/tx/detail/LoanSet.cpp b/src/xrpld/app/tx/detail/LoanSet.cpp index d9136f8051..7889192959 100644 --- a/src/xrpld/app/tx/detail/LoanSet.cpp +++ b/src/xrpld/app/tx/detail/LoanSet.cpp @@ -282,6 +282,15 @@ LoanSet::preclaim(PreclaimContext const& ctx) if (!vault) // Should be impossible return tefBAD_LEDGER; // LCOV_EXCL_LINE + + if (vault->at(sfAssetsMaximum) != 0 && + vault->at(sfAssetsTotal) >= vault->at(sfAssetsMaximum)) + { + JLOG(ctx.j.warn()) + << "Vault at maximum assets limit. Can't add another loan."; + return tecLIMIT_EXCEEDED; + } + Asset const asset = vault->at(sfAsset); auto const vaultPseudo = vault->at(sfAccount); @@ -411,8 +420,12 @@ LoanSet::doApply() principalRequested, properties.loanState.managementFeeDue); - if (vaultSle->at(sfAssetsMaximum) != 0 && - vaultTotalProxy + state.interestDue > vaultSle->at(sfAssetsMaximum)) + auto const vaultMaximum = *vaultSle->at(sfAssetsMaximum); + XRPL_ASSERT_PARTS( + vaultMaximum == 0 || vaultMaximum > *vaultTotalProxy, + "ripple::LoanSet::doApply", + "Vault is below maximum limit"); + if (vaultMaximum != 0 && state.interestDue > vaultMaximum - vaultTotalProxy) { JLOG(j_.warn()) << "Loan would exceed the maximum assets of the vault"; return tecLIMIT_EXCEEDED; @@ -600,7 +613,7 @@ LoanSet::doApply() loan->at(sfTotalValueOutstanding) = properties.loanState.valueOutstanding; loan->at(sfManagementFeeOutstanding) = properties.loanState.managementFeeDue; - loan->at(sfPreviousPaymentDate) = 0; + loan->at(sfPreviousPaymentDueDate) = 0; loan->at(sfNextPaymentDueDate) = startDate + paymentInterval; loan->at(sfPaymentRemaining) = paymentTotal; view.insert(loan);