Bring back the Rust XDR parsing check. (stellar#5396)

# Description

This check has been removed in commit
`4287224758c47dc6cdc0c14930754c898b886e51`, however it's still necessary
for most of the Core builds that support 2 protocol version.

This change reverts the removal commit and does a bit of necessary
refactoring to support the new build.

# Checklist
- [ ] Reviewed the
[contributing](https://github.com/stellar/stellar-core/blob/master/CONTRIBUTING.md#submitting-changes)
document
- [ ] Rebased on top of master (no merge commits)
- [ ] Ran `clang-format` v8.0.0 (via `make format` or the Visual Studio
extension)
- [ ] Compiles
- [ ] Ran all tests
- [ ] If change impacts performance, include supporting evidence per the
[performance
document](https://github.com/stellar/stellar-core/blob/master/performance-eval/performance-eval.md)

Author: graydon

src/test/TestUtils.cpp

@@ -9,6 +9,7 @@
 #include "rust/RustBridge.h"
 #include "simulation/LoadGenerator.h"
 #include "simulation/Simulation.h"
+#include "test/Catch2.h"
 #include "test/TxTests.h"
 #include "test/test.h"
 #include "transactions/test/SorobanTxTestUtils.h"
@@ -632,4 +633,21 @@ generateTransactions(Application& app, std::filesystem::path const& outputFile,
     LOG_INFO(DEFAULT_LOG, "Generated {} transactions in {}", numTransactions,
              outputFile);
 }
+
+bool
+isSorobanProtocolLinked(Config const& cfg, ProtocolVersion protocolVersion)
+{
+    auto sorobanProtocolCfg = cfg;
+    sorobanProtocolCfg.USE_CONFIG_FOR_GENESIS = true;
+    sorobanProtocolCfg.TESTING_UPGRADE_LEDGER_PROTOCOL_VERSION =
+        static_cast<uint32_t>(protocolVersion);
+    auto res =
+        testutil::isTestApplicationProtocolVersionSupported(sorobanProtocolCfg);
+    if (!res)
+    {
+        SUCCEED("Skipping historical Soroban protocol test: requested "
+                "protocol is not linked in this build");
+    }
+    return res;
+}
 }

src/test/TestUtils.h

@@ -146,4 +146,6 @@ void generateTransactions(Application& app,
                           std::filesystem::path const& outputFile,
                           uint32_t numTransactions, uint32_t accounts,
                           uint32_t offset);
+bool isSorobanProtocolLinked(Config const& cfg,
+                             ProtocolVersion protocolVersion);
 }

src/transactions/FeeBumpTransactionFrame.cpp

@@ -277,7 +277,9 @@ FeeBumpTransactionFrame::checkValidImpl(
     DiagnosticEventManager& diagnosticEvents, bool isOverlayValidation,
     std::optional<uint32_t> validationLedgerSeq) const
 {
-    if (!xdr::check_xdr_depth(mEnvelope, 500) || !XDRProvidesValidFee())
+    auto ledgerVersion = ledgerView.getLedgerHeader().current().ledgerVersion;
+    if (!validateXDRForProtocol(ledgerVersion, app.getConfig(), mEnvelope) ||
+        !XDRProvidesValidFee())
     {
         return FeeBumpMutableTransactionResult::createTxError(txMALFORMED);
     }
@@ -292,7 +294,6 @@ FeeBumpTransactionFrame::checkValidImpl(
     auto txResult = FeeBumpMutableTransactionResult::createSuccess(
         *mInnerTx, feeCharged, 0);
 
-    auto ledgerVersion = ledgerView.getLedgerHeader().current().ledgerVersion;
     SignatureChecker signatureChecker{ledgerVersion, getContentsHash(),
                                       mEnvelope.feeBump().signatures,
                                       isOverlayValidation};

src/transactions/TransactionFrame.cpp

@@ -1934,11 +1934,13 @@ TransactionFrame::checkValidImpl(
     DiagnosticEventManager& diagnosticEvents, bool isOverlayValidation,
     std::optional<uint32_t> validationLedgerSeq) const
 {
+    auto const& header = ledgerView.getLedgerHeader().current();
     // Subtle: this check has to happen in `checkValid` and not
     // `checkValidWithOptionallyChargedFee` in order to not validate the
     // envelope XDR twice for the fee bump transactions (they use
     // `checkValidWithOptionallyChargedFee` for the inner tx).
-    if (!xdr::check_xdr_depth(mEnvelope, 500))
+    if (!validateXDRForProtocol(header.ledgerVersion, app.getConfig(),
+                                mEnvelope))
     {
         return MutableTransactionResult::createTxError(txMALFORMED);
     }
@@ -1952,9 +1954,8 @@ TransactionFrame::checkValidImpl(
     // aren't the fees that would end up being applied. However, this is
     // what Core used to return for a while, and some users may rely on
     // this, so we maintain this logic for the time being.
-    int64_t minBaseFee = ledgerView.getLedgerHeader().current().baseFee;
-    auto feeCharged =
-        getFee(ledgerView.getLedgerHeader().current(), minBaseFee, false);
+    int64_t minBaseFee = header.baseFee;
+    auto feeCharged = getFee(header, minBaseFee, false);
     auto txResult = MutableTransactionResult::createSuccess(*this, feeCharged);
     checkValidWithOptionallyChargedFee(
         app, ledgerView, current, true, lowerBoundCloseTimeOffset,

src/transactions/TransactionUtils.cpp

@@ -1345,6 +1345,39 @@ hasMuxedAccount(TransactionEnvelope const& e)
     return c.mHasMuxedAccount;
 }
 
+bool
+validateXDRForProtocol(uint32_t currProtocol, Config const& cfg,
+                       TransactionEnvelope const& envelope)
+{
+    uint32_t rustDepthLimit = 1000;
+    // Rust XDR parser may measure depth to be deeper than depth measured by
+    // the C++ parser. Before p28 we didn't do the Rust parsing check at all
+    // and instead relied on the C++ check with a reduced depth limit to
+    // approximate for that fact.
+    // Starting with p28 build we do the Rust parsing check unconditionally both
+    // to ensure that XDR can be parsed with the parser for the given protocol
+    // version, and to ensure that the depth limit is not exceeded. However, in
+    // order to avoid divergence with p27 builds, we still do a C++ depth check
+    // and relax the Rust depth limit (so that it's a basically no-op) in order
+    // to ensure identical behavior in both builds.
+    // Note, that the actual 50'000 depth won't be reachable in practice, so we
+    // don't need to worry about the stack overflow in the Rust parser.
+    if (protocolVersionIsBefore(currProtocol, ProtocolVersion::V_28))
+    {
+        if (!xdr::check_xdr_depth(envelope, 500))
+        {
+            return false;
+        }
+        rustDepthLimit = 50'000;
+    }
+
+    uint32_t maxProtocol = cfg.CURRENT_LEDGER_PROTOCOL_VERSION;
+    auto cxxBuf = CxxBuf{
+        std::make_unique<std::vector<uint8_t>>(xdr::xdr_to_opaque(envelope))};
+    return rust_bridge::can_parse_transaction(maxProtocol, currProtocol, cxxBuf,
+                                              rustDepthLimit);
+}
+
 uint64_t
 getUpperBoundCloseTimeOffset(Application& app, uint64_t lastCloseTime)
 {

src/transactions/TransactionUtils.h

@@ -264,6 +264,13 @@ bool accountFlagMaskCheckIsValid(uint32_t flag, uint32_t ledgerVersion);
 
 bool hasMuxedAccount(TransactionEnvelope const& e);
 
+// Checks if the transaction XDR is parseable for the provided protocol version.
+// The 'parse-ability' check ensures that the XDR is not too deeply nested, and
+// that a Soroban host for the provided protocol version would be able to parse
+// the XDR.
+bool validateXDRForProtocol(uint32_t currProtocol, Config const& cfg,
+                            TransactionEnvelope const& envelope);
+
 uint64_t getUpperBoundCloseTimeOffset(Application& app, uint64_t lastCloseTime);
 
 bool hasAccountEntryExtV2(AccountEntry const& ae);

src/transactions/test/InvokeHostFunctionTests.cpp

@@ -89,17 +89,6 @@ getParallelSorobanTestProtocolVersion()
     return testLedgerProtocolVersion;
 }
 
-bool
-isSorobanProtocolLinked(Config const& cfg, ProtocolVersion protocolVersion)
-{
-    auto sorobanProtocolCfg = cfg;
-    sorobanProtocolCfg.USE_CONFIG_FOR_GENESIS = true;
-    sorobanProtocolCfg.TESTING_UPGRADE_LEDGER_PROTOCOL_VERSION =
-        static_cast<uint32_t>(protocolVersion);
-    return testutil::isTestApplicationProtocolVersionSupported(
-        sorobanProtocolCfg);
-}
-
 void
 overrideNetworkSettingsToMin(Application& app)
 {
@@ -7222,8 +7211,6 @@ TEST_CASE("Module cache", "[tx][soroban]")
     cfg.USE_CONFIG_FOR_GENESIS = false;
     if (!isSorobanProtocolLinked(cfg, SOROBAN_PROTOCOL_VERSION))
     {
-        SUCCEED("Skipping historical Soroban protocol test: requested "
-                "protocol is not linked in this build");
         return;
     }
 
@@ -7283,8 +7270,6 @@ TEST_CASE("Vm instantiation tightening", "[tx][soroban]")
     cfg.USE_CONFIG_FOR_GENESIS = false;
     if (!isSorobanProtocolLinked(cfg, SOROBAN_PROTOCOL_VERSION))
     {
-        SUCCEED("Skipping historical Soroban protocol test: requested "
-                "protocol is not linked in this build");
         return;
     }
 

src/transactions/test/TxEnvelopeTests.cpp

@@ -3049,3 +3049,134 @@ TEST_CASE_VERSIONS("soroban transaction validation", "[tx][envelope][soroban]")
             }
         });
 }
+
+TEST_CASE("XDR protocol 22 compatibility validation", "[tx][envelope]")
+{
+    if (!isSorobanProtocolLinked(getTestConfig(), ProtocolVersion::V_21))
+    {
+        return;
+    }
+    auto validateTx = [](ProtocolVersion protocolVersion) {
+        VirtualClock clock;
+        auto cfg = getTestConfig();
+        cfg.TESTING_UPGRADE_LEDGER_PROTOCOL_VERSION =
+            static_cast<uint32_t>(protocolVersion);
+        auto app = createTestApplication(clock, cfg);
+        auto root = app->getRoot();
+        Operation op;
+        op.body.type(INVOKE_HOST_FUNCTION);
+        op.body.invokeHostFunctionOp().hostFunction.type(
+            HOST_FUNCTION_TYPE_CREATE_CONTRACT_V2);
+
+        auto tx =
+            sorobanTransactionFrameFromOps(app->getNetworkID(), *root, {op}, {},
+                                           SorobanResources(), 1000, 1'000'000);
+        CheckValidLedgerViewWrapper ledgerView(*app);
+        return tx->checkValid(app->getAppConnector(), ledgerView, 0, 0, 0);
+    };
+    SECTION("not valid in protocol 21")
+    {
+        auto res = validateTx(ProtocolVersion::V_21);
+        REQUIRE(res->getResultCode() == txMALFORMED);
+    }
+    SECTION("valid in protocol 22")
+    {
+        auto res = validateTx(ProtocolVersion::V_22);
+        REQUIRE(res->isSuccess());
+    }
+}
+
+TEST_CASE("XDR protocol 23 compatibility validation", "[tx][envelope]")
+{
+    if (!isSorobanProtocolLinked(getTestConfig(), ProtocolVersion::V_22))
+    {
+        return;
+    }
+    auto runTest = [](ProtocolVersion protocolVersion, bool expectSuccess) {
+        VirtualClock clock;
+        auto cfg = getTestConfig();
+        cfg.TESTING_UPGRADE_LEDGER_PROTOCOL_VERSION =
+            static_cast<uint32_t>(protocolVersion);
+        auto app = createTestApplication(clock, cfg);
+        auto root = app->getRoot();
+        Operation op;
+        op.body.type(INVOKE_HOST_FUNCTION);
+        op.body.invokeHostFunctionOp().hostFunction.type(
+            HOST_FUNCTION_TYPE_INVOKE_CONTRACT);
+
+        CheckValidLedgerViewWrapper ledgerView(*app);
+        SECTION("muxed account ScAddress in function args")
+        {
+            auto& val = op.body.invokeHostFunctionOp()
+                            .hostFunction.invokeContract()
+                            .args.emplace_back();
+            val.type(SCV_ADDRESS);
+            val.address().type(SC_ADDRESS_TYPE_MUXED_ACCOUNT);
+            val.address().muxedAccount().id = 123;
+            auto tx = sorobanTransactionFrameFromOps(
+                app->getNetworkID(), *root, {op}, {}, SorobanResources(), 1000,
+                1'000'000);
+
+            auto res =
+                tx->checkValid(app->getAppConnector(), ledgerView, 0, 0, 0);
+            REQUIRE(res->isSuccess() == expectSuccess);
+            if (!expectSuccess)
+            {
+                REQUIRE(res->getResultCode() == txMALFORMED);
+            }
+        }
+        SECTION("claimable balance ScAddress in auth")
+        {
+            auto& authEntry =
+                op.body.invokeHostFunctionOp().auth.emplace_back();
+            authEntry.rootInvocation.function.type(
+                SOROBAN_AUTHORIZED_FUNCTION_TYPE_CONTRACT_FN);
+            auto& address =
+                authEntry.rootInvocation.function.contractFn().contractAddress;
+            address.type(SC_ADDRESS_TYPE_CLAIMABLE_BALANCE);
+            address.claimableBalanceId().v0()[0] = 1;
+            auto tx = sorobanTransactionFrameFromOps(
+                app->getNetworkID(), *root, {op}, {}, SorobanResources(), 1000,
+                1'000'000);
+            auto res =
+                tx->checkValid(app->getAppConnector(), ledgerView, 0, 0, 0);
+            REQUIRE(res->isSuccess() == expectSuccess);
+            if (!expectSuccess)
+            {
+                REQUIRE(res->getResultCode() == txMALFORMED);
+            }
+        }
+        SECTION("liquidity pool ScAddress in footprint")
+        {
+            Operation ttlOp;
+            ttlOp.body.type(EXTEND_FOOTPRINT_TTL);
+
+            SorobanResources resources;
+            auto& key = resources.footprint.readOnly.emplace_back();
+            key.type(CONTRACT_DATA);
+            auto& address = key.contractData().contract;
+
+            address.type(SC_ADDRESS_TYPE_LIQUIDITY_POOL);
+            address.liquidityPoolId()[1] = 10;
+            auto tx = sorobanTransactionFrameFromOps(app->getNetworkID(), *root,
+                                                     {ttlOp}, {}, resources,
+                                                     1000, 1'000'000);
+            auto res =
+                tx->checkValid(app->getAppConnector(), ledgerView, 0, 0, 0);
+            REQUIRE(res->isSuccess() == expectSuccess);
+            if (!expectSuccess)
+            {
+                REQUIRE(res->getResultCode() == txMALFORMED);
+            }
+        }
+    };
+
+    SECTION("not valid in protocol 22")
+    {
+        runTest(ProtocolVersion::V_22, false);
+    }
+    SECTION("valid in protocol 23")
+    {
+        runTest(ProtocolVersion::V_23, true);
+    }
+}