Merge branch 'develop' into ximinez/fix-batchinnersigs

This change updates a script in the documentation to automatically infer the version of a patched Conan dependency from the conan.lock file.

BUILD.md

@@ -141,26 +141,37 @@ Alternatively, you can pull the patched recipes into the repository and use them
 locally:
 
 ```bash
+# Extract the version number from the lockfile.
+function extract_version {
+  version=$(cat conan.lock | sed -nE "s@.+${1}/(.+)#.+@\1@p" | head -n1)
+  echo ${version}
+}
+
+# Define which recipes to export.
+recipes=(ed25519 grpc secp256k1 snappy soci)
+
+# Selectively check out the recipes from our CCI fork.
 cd external
 mkdir -p conan-center-index
 cd conan-center-index
 git init
 git remote add origin git@github.com:XRPLF/conan-center-index.git
 git sparse-checkout init
-git sparse-checkout set recipes/ed25519
-git sparse-checkout add recipes/grpc
-git sparse-checkout add recipes/secp256k1
-git sparse-checkout add recipes/snappy
-git sparse-checkout add recipes/soci
+for recipe in ${recipes[@]}; do
+  echo "Checking out ${recipe}..."
+  git sparse-checkout add recipes/${recipe}/all
+done
 git fetch origin master
 git checkout master
-rm -rf .git
 cd ../..
-conan export --version 2015.03 external/conan-center-index/recipes/ed25519/all
-conan export --version 1.72.0 external/conan-center-index/recipes/grpc/all
-conan export --version 0.7.0 external/conan-center-index/recipes/secp256k1/all
-conan export --version 1.1.10 external/conan-center-index/recipes/snappy/all
-conan export --version 4.0.3 external/conan-center-index/recipes/soci/all
+
+# Export the recipes into the local cache.
+for recipe in ${recipes[@]}; do
+  version=$(extract_version ${recipe})
+  echo "Exporting ${recipe}/${version}..."
+  conan export --version $(extract_version ${recipe}) \
+    external/conan-center-index/recipes/${recipe}/all
+done
 ```
 
 In the case we switch to a newer version of a dependency that still requires a

include/xrpl/protocol/detail/features.macro

@@ -16,6 +16,7 @@
 // Add new amendments to the top of this list.
 // Keep it sorted in reverse chronological order.
 
+XRPL_FIX    (BatchInnerSigs,             Supported::yes, VoteBehavior::DefaultNo)
 XRPL_FEATURE(LendingProtocol,            Supported::no,  VoteBehavior::DefaultNo)
 XRPL_FEATURE(PermissionDelegationV1_1,   Supported::no,  VoteBehavior::DefaultNo)
 XRPL_FIX    (DirectoryLimit,             Supported::yes, VoteBehavior::DefaultNo)

src/test/app/Batch_test.cpp

@@ -2295,9 +2295,12 @@ class Batch_test : public beast::unit_test::suite
             Serializer s;
             parsed.object->add(s);
             auto const jrr = env.rpc("submit", strHex(s.slice()))[jss::result];
-            BEAST_EXPECT(
-                jrr[jss::status] == "success" &&
-                jrr[jss::engine_result] == "temINVALID_FLAG");
+            BEAST_EXPECTS(
+                jrr[jss::status] == "error" &&
+                    jrr[jss::error] == "invalidTransaction" &&
+                    jrr[jss::error_exception] ==
+                        "fails local checks: Empty SigningPubKey.",
+                to_string(jrr));
 
             env.close();
         }
@@ -4353,6 +4356,7 @@ public:
         using namespace test::jtx;
         auto const sa = testable_amendments();
         testWithFeats(sa);
+        testWithFeats(sa - fixBatchInnerSigs);
     }
 };
 

src/xrpld/app/misc/NetworkOPs.cpp

@@ -1681,7 +1681,7 @@ NetworkOPsImp::apply(std::unique_lock<std::mutex>& batchLock)
                     // only be set if the Batch feature is enabled. If Batch is
                     // not enabled, the flag is always invalid, so don't relay
                     // it regardless.
-                    !sttx.isFlag(tfInnerBatchTxn))
+                    !(sttx.isFlag(tfInnerBatchTxn)))
                 {
                     protocol::TMTransaction tx;
                     Serializer s;

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

@@ -671,7 +671,10 @@ Transactor::checkSign(
 
     auto const pkSigner = sigObject.getFieldVL(sfSigningPubKey);
     // Ignore signature check on batch inner transactions
-    if (parentBatchId && view.rules().enabled(featureBatch))
+    bool const useCtx = view.rules().enabled(fixBatchInnerSigs);
+    if ((useCtx ? parentBatchId.has_value()
+                : sigObject.isFlag(tfInnerBatchTxn)) &&
+        view.rules().enabled(featureBatch))
     {
         // Defensive Check: These values are also checked in Batch::preflight
         if (sigObject.isFieldPresent(sfTxnSignature) || !pkSigner.empty() ||

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

@@ -41,15 +41,22 @@ checkValidity(
                 Validity::SigBad,
                 "Malformed: Invalid inner batch transaction."};
 
-        std::string reason;
-        if (!passesLocalChecks(tx, reason))
+        // This block should probably have never been included in the
+        // original `Batch` implementation. An inner transaction never
+        // has a valid signature.
+        bool const neverValid = rules.enabled(fixBatchInnerSigs);
+        if (!neverValid)
         {
-            router.setFlags(id, SF_LOCALBAD);
-            return {Validity::SigGoodOnly, reason};
-        }
+            std::string reason;
+            if (!passesLocalChecks(tx, reason))
+            {
+                router.setFlags(id, SF_LOCALBAD);
+                return {Validity::SigGoodOnly, reason};
+            }
 
-        router.setFlags(id, SF_SIGGOOD);
-        return {Validity::Valid, ""};
+            router.setFlags(id, SF_SIGGOOD);
+            return {Validity::Valid, ""};
+        }
     }
 
     if (any(flags & SF_SIGBAD))