Merge branch 'develop' into ximinez/sync-script

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

.github/workflows/reusable-package.yml

@@ -58,6 +58,7 @@ jobs:
 
   package:
     needs: [generate-matrix, generate-version]
+    if: ${{ github.event.repository.visibility == 'public' }}
     strategy:
       fail-fast: false
       matrix: ${{ fromJson(needs.generate-matrix.outputs.matrix) }}
@@ -88,8 +89,7 @@ jobs:
         run: ./package/build_pkg.sh
 
       - name: Upload package artifact
-        uses: actions/upload-artifact@bbbca2ddaa5d8feaa63e36b76fdaad77386f024f # v7.0.0
-        if: ${{ github.event.repository.visibility == 'public' }}
+        uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1
         with:
           name: ${{ matrix.artifact_name }}-pkg-${{ needs.generate-version.outputs.version }}
           path: |

bin/git/sync-to-release.sh

@@ -0,0 +1,159 @@
+#!/bin/bash -e
+
+set -o pipefail
+
+_usage()
+{
+    self=$( basename -- $( echo $0 ) )
+    cat << USAGE
+Usage: $self release_branch develop_branch working_branch [new_version]
+
+This script is intended to be used to synchronize changes between two
+branches where the "release_branch" has a few insignificant changes (e.g.
+version numbers) compared to the "develop_branch", which has active
+ongoing development. It has not (yet) been tested in scenarios where the
+branches have significantly diverged, or where one has been merged to
+the other.
+
+release_branch: Usually a pending release branch, this is the low
+    activity branch which needs to be updated from develop_branch.
+
+develop_branch: The active development branch.
+
+working_branch: This branch will be created from the release_branch to
+    have the new commits applied. If it exists, it will be assumed to have
+    been created by an earlier invocation, and will be used for
+    comparisons instead of release_branch.  When the script is done, this
+    branch will NOT be pushed. The user must manually push the branch and
+    create a pull request.
+
+new_version: OPTIONAL. If provided, will update the version string in
+    BuildInfo.cpp to this value, and create a version_setting commit.
+
+USAGE
+    if [[ $# -ne 0 ]]; then
+        echo "Error: ${@}"
+        echo
+    fi
+
+    exit 1
+}
+
+if [[ $# -lt 3 || $# -gt 4 ]]; then
+    _usage
+fi
+
+release_branch="$1"
+shift
+
+develop_branch="$1"
+shift
+
+working_branch="$1"
+shift
+
+if [[ $# -ne 0 ]]; then
+    new_version="$1"
+    shift
+fi
+
+_run()
+{
+    echo
+    echo "\$ ${@}"
+    "${@}"
+}
+
+if git rev-parse --quiet --verify --end-of-options "refs/heads/${working_branch}" > /dev/null; then
+    release_branch="${working_branch}"
+    _run git checkout "${working_branch}"
+else
+    _run git checkout --no-track -B "${working_branch}" "${release_branch}"
+fi
+
+
+# Fetch if it hasn't been done in a while
+if find "$( git rev-parse --git-dir )/FETCH_HEAD" -type f -mmin +10 | grep -q "FETCH"; then
+    _run git fetch --all
+fi
+
+temp=$( mktemp )
+
+# The "word-diff" option doesn't seem to have any meaningful effect on the output.
+# This may make this script fragile across git versions with differing output formats.
+_run git range-diff --word-diff=porcelain --no-patch --right-only \
+    "${release_branch}"..."${develop_branch}" | tee "${temp}"
+
+first_commit=$( cat "${temp}" | \
+    awk '( $1 == "-:" && $2 ~ /-+/ && $3 == ">" ) { print $5; exit; }'  )
+
+if [[ "${first_commit}" == "" && "${new_version}" == "" ]]; then
+    _usage No divergence found
+    exit 1
+fi
+
+if [[ "${first_commit}" == "" ]]; then
+    echo -e "\nNo divergence found. Continue to update version number"
+else
+    echo -e "\nFound first unique commit in ${develop_branch}: ${first_commit}"
+
+    first_commit=$( git rev-parse --quiet "${first_commit}" )
+
+    echo "Full commit ID is: ${first_commit}"
+
+    merge_base=$( git merge-base --octopus "${first_commit}" "${develop_branch}" )
+    if [[ "${first_commit}" != "${merge_base}" ]]; then
+        echo "Bad first commit: ${first_commit} is not an ancestor of ${develop_branch}"
+        exit 1
+    fi
+
+    _run git diff "${first_commit}~" "${release_branch}"
+
+    read -n 1 -s -p "Does the diff look reasonable? (y to continue / any other key to abort)" continue
+    echo
+    if [[ ! ( "${continue}" =~ [yY] ) ]]; then
+        echo Abort
+        exit 1
+    fi
+
+    # Dilemma: if the cherry-pick is interrupted, for example, because of a conflict, the script
+    # will abort, and can not easily be resumed. This will prevent the final step(s) from running.
+    if ! _run git cherry-pick --empty=drop "${first_commit}~..${develop_branch}"; then
+        # Solution: if the cherry-pick is interrupted, run the command again.
+        if [[ "${new_version}" != "" ]]; then
+            echo
+            echo "Cherry pick failed or was interrupted. Resolve issues, finish the"
+            echo "  cherry pick, then run this script again to with the same parameters"
+            echo "  to update the version number."
+        fi
+        exit 1
+    fi
+
+    if [[ "${new_version}" == "" ]]; then
+        # If there is no version number change to make, we're done.
+        exit 0
+    fi
+fi
+
+# There should only be one matching file, but loop in case not.
+buildfiles=( $( git ls-files | grep "BuildInfo.cpp" ) )
+
+for build in "${buildfiles[@]}"; do
+    sed 's/\(^.*versionString =\).*$/\1 "'${new_version}'"/' ${build} > version.cpp && \
+    _run diff "${build}" version.cpp && exit 1 || \
+    mv -vi version.cpp ${build}
+
+    git add ${build}
+done
+
+_run git commit -S -m "Set version to ${new_version}"
+
+_run git log --oneline --first-parent ${release_branch}^..
+
+cat << PUSH
+
+-------------------------------------------------------------------
+This script will not push to remote. Verify everything is correct,
+then push, and create a PR as described in CONTRIBUTING.md.
+-------------------------------------------------------------------
+PUSH

src/libxrpl/tx/transactors/vault/VaultDeposit.cpp

@@ -7,6 +7,7 @@
 #include <xrpl/ledger/helpers/MPTokenHelpers.h>
 #include <xrpl/ledger/helpers/TokenHelpers.h>
 #include <xrpl/ledger/helpers/VaultHelpers.h>
+#include <xrpl/protocol/Feature.h>
 #include <xrpl/protocol/Indexes.h>
 #include <xrpl/protocol/Issue.h>
 #include <xrpl/protocol/LedgerFormats.h>
@@ -252,19 +253,26 @@ VaultDeposit::doApply()
         !isTesSuccess(ter))
         return ter;
 
-    // Sanity check
-    if (accountHolds(
-            view(),
-            accountID_,
-            assetsDeposited.asset(),
-            FreezeHandling::IgnoreFreeze,
-            AuthHandling::IgnoreAuth,
-            j_) < beast::kZero)
+    // This check is wrong. Disable it with fixCleanup3_2_0.
+    // For XRP and MPT the predicate is structurally unsatisfiable: xrpLiquid clamps at zero, and
+    // MPT balances are unsigned. For IOUs it only fires when the deposit drove the depositor's
+    // trust line into debt the exact case preclaim authorizes via SpendableHandling::FullBalance.
+    // The check thus converts a preclaim- authorized deposit into tefINTERNAL after the asset
+    // transfer.
+    if (!view().rules().enabled(fixCleanup3_2_0))
     {
-        // LCOV_EXCL_START
-        JLOG(j_.error()) << "VaultDeposit: negative balance of account assets.";
-        return tefINTERNAL;
-        // LCOV_EXCL_STOP
+        // Sanity check
+        if (accountHolds(
+                view(),
+                accountID_,
+                assetsDeposited.asset(),
+                FreezeHandling::IgnoreFreeze,
+                AuthHandling::IgnoreAuth,
+                j_) < beast::kZero)
+        {
+            JLOG(j_.error()) << "VaultDeposit: negative balance of account assets.";
+            return tefINTERNAL;
+        }
     }
 
     // Transfer shares from vault to depositor.

src/test/app/Vault_test.cpp

@@ -6140,10 +6140,90 @@ class Vault_test : public beast::unit_test::Suite
         runTest(amendments);
     }
 
+    // VaultDeposit::preclaim uses accountHolds(..., SpendableHandling::
+    // shFULL_BALANCE), which for an IOU asset adds the counterparty's
+    // LowLimit/HighLimit to the depositor's raw balance (TokenHelpers.cpp:
+    // getTrustLineBalance with includeOppositeLimit=true). When the
+    // depositor's raw balance < deposit amount but raw + opposite limit >=
+    // amount, preclaim is satisfied. doApply then calls
+    // directSendNoFeeIOU, which unconditionally subtracts saAmount from
+    // saBalance — driving the trust line negative — and returns tesSUCCESS.
+    // The post-send sanity check uses the default shSIMPLE_BALANCE (no
+    // opposite-limit add), sees a negative balance, and returns tefINTERNAL.
+    void
+    testVaultDepositNegativeBalanceFromOppositeLimit()
+    {
+        auto runTest = [&](FeatureBitset f, TER expected) {
+            using namespace test::jtx;
+            using namespace std::literals;
+
+            Env env{*this, f};
+            Account const gw{"gateway"};
+            Account const owner{"owner"};
+            Account const depositor{"depositor"};
+
+            env.fund(XRP(10000), gw, owner, depositor);
+            env.close();
+
+            // Gateway with DefaultRipple so vault creation on its IOU works.
+            env(fset(gw, asfDefaultRipple));
+            env.close();
+
+            // Depositor opens a trust line to gateway and receives a small
+            // balance.
+            PrettyAsset const usd = gw["USD"];
+            env.trust(usd(1000), depositor);
+            env(pay(gw, depositor, usd(100)));  // raw trust-line balance: 100
+            env.close();
+
+            // Key precondition: gateway sets a non-zero limit on the same
+            // RippleState — the "opposite field" from depositor's perspective.
+            // This is what inflates shFULL_BALANCE in preclaim above the raw
+            // balance.
+            env(trust(gw, depositor["USD"](1000)));
+            env.close();
+
+            // Create the IOU vault.
+            Vault const vault{env};
+            auto [vaultTx, keylet] = vault.create({.owner = owner, .asset = usd});
+            env(vaultTx);
+            env.close();
+
+            // Submit a deposit of 500 USD:
+            //   - raw balance:                100 USD
+            //   - opposite limit (gw's side): 1000 USD
+            //   - preclaim sees 100 + 1000 = 1100, passes (>= 500)
+            //   - doApply transfers 500, depositor's trust-line balance
+            //     becomes -400
+            //   - sanity check at VaultDeposit.cpp:256 fires
+            //   - tx returns tefINTERNAL (BUG — should be tesSUCCESS.
+            auto depositTx =
+                vault.deposit({.depositor = depositor, .id = keylet.key, .amount = usd(500)});
+            env(depositTx, Ter(expected));
+            env.close();
+        };
+
+        {
+            testcase(
+                "IOU vault deposit exceeding depositor's balance but "
+                "within counterparty's trust limit, pre-fixCleanup3_2_0 "
+                "(tefINTERNAL)");
+            runTest(test::jtx::testableAmendments() - fixCleanup3_2_0, tefINTERNAL);
+        }
+        {
+            testcase(
+                "IOU vault deposit exceeding depositor's balance but "
+                "within counterparty's trust limit, post-fixCleanup3_2_0 "
+                "(tesSUCCESS)");
+            runTest(test::jtx::testableAmendments(), tesSUCCESS);
+        }
+    }
+
 public:
     void
     run() override
     {
+        testVaultDepositNegativeBalanceFromOppositeLimit();
         testSequences();
         testPreflight();
         testCreateFailXRP();