From 41fc73a492d34868a7106968cc4e126d022186af Mon Sep 17 00:00:00 2001 From: Dmytro Kozhevin Date: Thu, 13 Mar 2025 13:59:28 -0400 Subject: [PATCH] Adapt Core to the CAP-67 XDR. This is unfortunately rather hacky in order to allow for these changes to go in a minor release. Specifically, we're only bumping XDR for the Core itself, and not for the Rust env. That's why we need to temporarily disable a few XDR integrity checks. There is also a point of interaction of the new and old XDR(ScAddress changes), and I've added some tests to ensure that Core won't be able to accidentally accept XDR that's not supported by the current protocol. --- Builds/VisualStudio/stellar-core.vcxproj | 15 ++++- .../VisualStudio/stellar-core.vcxproj.filters | 45 ++++++++++--- src/bucket/DiskIndex.cpp | 2 +- src/bucket/LiveBucketIndex.cpp | 2 +- src/main/main.cpp | 14 +++- src/protocol-curr/xdr | 2 +- src/protocol-next/xdr | 2 +- src/transactions/TransactionUtils.cpp | 9 ++- src/transactions/test/TxEnvelopeTests.cpp | 64 +++++++++++++++++++ 9 files changed, 136 insertions(+), 19 deletions(-) diff --git a/Builds/VisualStudio/stellar-core.vcxproj b/Builds/VisualStudio/stellar-core.vcxproj index 129a51ce82..7774021c9f 100644 --- a/Builds/VisualStudio/stellar-core.vcxproj +++ b/Builds/VisualStudio/stellar-core.vcxproj @@ -457,7 +457,7 @@ exit /b 0 - + @@ -467,10 +467,14 @@ exit /b 0 + + + + @@ -565,6 +569,7 @@ exit /b 0 + @@ -912,8 +917,7 @@ exit /b 0 - - + @@ -923,11 +927,15 @@ exit /b 0 + + + + @@ -1013,6 +1021,7 @@ exit /b 0 + diff --git a/Builds/VisualStudio/stellar-core.vcxproj.filters b/Builds/VisualStudio/stellar-core.vcxproj.filters index 5f2d5519d3..2429fb91fb 100644 --- a/Builds/VisualStudio/stellar-core.vcxproj.filters +++ b/Builds/VisualStudio/stellar-core.vcxproj.filters @@ -576,9 +576,6 @@ bucket - - bucket - bucket @@ -1365,6 +1362,24 @@ catchup + + bucket + + + bucket + + + bucket + + + bucket + + + bucket + + + history + @@ -1778,12 +1793,6 @@ bucket - - bucket - - - bucket - bucket @@ -2410,6 +2419,24 @@ catchup + + bucket + + + bucket + + + bucket + + + bucket + + + bucket + + + history + diff --git a/src/bucket/DiskIndex.cpp b/src/bucket/DiskIndex.cpp index 98be095e00..07c65fed31 100644 --- a/src/bucket/DiskIndex.cpp +++ b/src/bucket/DiskIndex.cpp @@ -133,7 +133,7 @@ DiskIndex::DiskIndex(BucketManager& bm, mData.assetToPoolID = std::make_unique(); } - auto fileSize = fs::size(filename); + auto fileSize = fs::size(filename.string()); auto estimatedIndexEntries = fileSize / pageSize; mData.keysToOffset.reserve(estimatedIndexEntries); diff --git a/src/bucket/LiveBucketIndex.cpp b/src/bucket/LiveBucketIndex.cpp index 90756808df..fa0756cd65 100644 --- a/src/bucket/LiveBucketIndex.cpp +++ b/src/bucket/LiveBucketIndex.cpp @@ -48,7 +48,7 @@ LiveBucketIndex::LiveBucketIndex(BucketManager& bm, ZoneScoped; releaseAssert(!filename.empty()); - auto pageSize = getPageSize(bm.getConfig(), fs::size(filename)); + auto pageSize = getPageSize(bm.getConfig(), fs::size(filename.string())); if (pageSize == 0) { diff --git a/src/main/main.cpp b/src/main/main.cpp index 2aca250880..96253b08ea 100644 --- a/src/main/main.cpp +++ b/src/main/main.cpp @@ -186,6 +186,18 @@ checkXDRFileIdentity() { continue; } + // Temporarily (until we have cut the next minor release) disable the + // check for the XDR changes necessary for the minor release. These + // are mostly binary compatible with the Rust XDR, and the incompatible + // part (ScAddress change) is actually supposed to be not decodable on + // the Rust side. + if (cpp.first.filename() == "Stellar-types.x" || + cpp.first.filename() == "Stellar-contract.x" || + cpp.first.filename() == "Stellar-ledger-entries.x" || + cpp.first.filename() == "Stellar-ledger.x") + { + continue; + } bool found = false; for (auto const& rust : rustHashes) { @@ -351,7 +363,7 @@ main(int argc, char* const* argv) rust_bridge::check_sensible_soroban_config_for_protocol( Config::CURRENT_LEDGER_PROTOCOL_VERSION); - // Disable XDR hash checking in vnext builds + // Temporarily disable XDR hash checking in vnext builds #ifndef ENABLE_NEXT_PROTOCOL_VERSION_UNSAFE_FOR_PRODUCTION checkXDRFileIdentity(); #endif diff --git a/src/protocol-curr/xdr b/src/protocol-curr/xdr index fa0338f4a2..515cb63a99 160000 --- a/src/protocol-curr/xdr +++ b/src/protocol-curr/xdr @@ -1 +1 @@ -Subproject commit fa0338f4a25a95320d2143c8f08d200f0a360a4b +Subproject commit 515cb63a99ad079519c59a35097b9c1e3d3deea1 diff --git a/src/protocol-next/xdr b/src/protocol-next/xdr index 9fc7936481..c9011cc6ca 160000 --- a/src/protocol-next/xdr +++ b/src/protocol-next/xdr @@ -1 +1 @@ -Subproject commit 9fc7936481a0ee43f015f940dda971f790c36862 +Subproject commit c9011cc6cae0c239908374c98000a72ab9bf895b diff --git a/src/transactions/TransactionUtils.cpp b/src/transactions/TransactionUtils.cpp index 15a177906b..63b4bba64c 100644 --- a/src/transactions/TransactionUtils.cpp +++ b/src/transactions/TransactionUtils.cpp @@ -1986,10 +1986,15 @@ isTransactionXDRValidForProtocol(uint32_t currProtocol, Config const& cfg, uint32_t maxProtocol = cfg.CURRENT_LEDGER_PROTOCOL_VERSION; // If we could parse the XDR when ledger is using the maximum supported // protocol version, then XDR has to be valid. + // This check is temporarily disabled because we're using different XDR in + // Rust and Core, and thus even at max protocol the check is necessary. + // if (maxProtocol == currProtocol) + //{ + // return true; + //} // This check also is pointless before protocol 21 as Soroban environment // doesn't support XDR versions before 21. - if (maxProtocol == currProtocol || - protocolVersionIsBefore(currProtocol, ProtocolVersion::V_21)) + if (protocolVersionIsBefore(currProtocol, ProtocolVersion::V_21)) { return true; } diff --git a/src/transactions/test/TxEnvelopeTests.cpp b/src/transactions/test/TxEnvelopeTests.cpp index 5964e6cdcd..3cfb711055 100644 --- a/src/transactions/test/TxEnvelopeTests.cpp +++ b/src/transactions/test/TxEnvelopeTests.cpp @@ -2531,6 +2531,70 @@ TEST_CASE("XDR protocol compatibility validation", "[tx][envelope]") } } +// This is a temporary case until we have released the next minor version. +// This can be safely removed after that (the test will also fail as soon as we +// caught up with the next XDR on the Rust side). +TEST_CASE("new ScAddress variants are not decodable by Rust", "[tx][envelope]") +{ + + VirtualClock clock; + auto cfg = getTestConfig(); + cfg.TESTING_UPGRADE_LEDGER_PROTOCOL_VERSION = + Config::CURRENT_LEDGER_PROTOCOL_VERSION; + auto app = createTestApplication(clock, cfg); + auto root = TestAccount::createRoot(*app); + Operation op; + op.body.type(INVOKE_HOST_FUNCTION); + op.body.invokeHostFunctionOp().hostFunction.type( + HOST_FUNCTION_TYPE_INVOKE_CONTRACT); + + LedgerSnapshot ls(*app); + SECTION("Invalid 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(), ls, 0, 0, 0); + REQUIRE(res->getResult().result.code() == txMALFORMED); + } + SECTION("Invalid 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(), ls, 0, 0, 0); + REQUIRE(res->getResult().result.code() == txMALFORMED); + } + SECTION("Invalid ScAddress in footprint") + { + 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, {op}, {}, resources, 1000, 1'000'000); + auto res = tx->checkValid(app->getAppConnector(), ls, 0, 0, 0); + REQUIRE(res->getResult().result.code() == txMALFORMED); + } +} + TEST_CASE_VERSIONS("Soroban extension for non-Soroban tx", "[tx][envelope][soroban]") {