More fixes to fee bump tx tests.

dmkozh · dmkozh · commit fd5935a93fea · 2025-04-29T18:30:49.000-04:00
The apply tests were doing something really weird: they used the result generated by `checkValidForTesting` and then proceeded to `apply`. This is not a flow that actually exists in Core - `apply` always uses the result provided by `processFeeSeqNum` and `checkValid` is completely separate from the apply flow. This issue resulted in observing the inner fee charged as 0 and not the inner tx fee charged, i.e. it hid the fact that we always set `feeCharged` in the inner result of fee bump txs.

Also updated the whole test to not use `checkValidForTesting` and removed an unnecessary argument from the event manager interface.
diff --git a/Builds/VisualStudio/generate_rust_bridge.bat b/Builds/VisualStudio/generate_rust_bridge.bat
@@ -10,13 +10,13 @@ set "final_source=%out_dir%\RustBridge.cpp"
 
 mkdir "%out_dir%\temp" >nul 2>nul
 
-%1 ..\..\src\rust\src\lib.rs --cfg test=false --header --output "%temp_header%"
+%1 ..\..\src\rust\src\bridge.rs --cfg test=false --header --output "%temp_header%"
 if %errorlevel% neq 0 (
     echo Error generating temporary header file.
     exit /b %errorlevel%
 )
 
-%1 ..\..\src\rust\src\lib.rs --cfg test=false --output "%temp_source%"
+%1 ..\..\src\rust\src\bridge.rs --cfg test=false --output "%temp_source%"
 if %errorlevel% neq 0 (
     echo Error generating temporary source file.
     exit /b %errorlevel%
diff --git a/src/ledger/LedgerManagerImpl.cpp b/src/ledger/LedgerManagerImpl.cpp
@@ -1826,7 +1826,7 @@ LedgerManagerImpl::applyTransactions(
 
             TxEventManager txEventManager(
                 ltx.loadHeader().current().ledgerVersion, mApp.getNetworkID(),
-                mApp.getConfig(), *tx);
+                mApp.getConfig());
             tx->apply(mApp.getAppConnector(), ltx, tm, mutableTxResult,
                       txEventManager, subSeed);
             tx->processPostApply(mApp.getAppConnector(), ltx, tm,
diff --git a/src/test/FuzzerImpl.cpp b/src/test/FuzzerImpl.cpp
@@ -927,7 +927,7 @@ class FuzzTransactionFrame : public TransactionFrame
         // reset results of operations
         mTxResult = createSuccessResultWithFeeCharged(ltx.getHeader(), 0, true);
         TxEventManager txEventManager(ltx.loadHeader().current().ledgerVersion,
-                                      mNetworkID, app.getConfig(), *this);
+                                      mNetworkID, app.getConfig());
 
         // attempt application of transaction without processing the fee or
         // committing the LedgerTxn
diff --git a/src/transactions/EventManager.cpp b/src/transactions/EventManager.cpp
@@ -556,12 +556,10 @@ OpEventManager::flushContractEvents(xdr::xvector<ContractEvent>& buf)
 };
 
 TxEventManager::TxEventManager(uint32_t protocolVersion, Hash const& networkID,
-                               Config const& config,
-                               TransactionFrameBase const& tx)
+                               Config const& config)
     : mProtocolVersion(protocolVersion)
     , mNetworkID(networkID)
     , mConfig(config)
-    , mTx(tx)
     , mDiagnosticEvents(DiagnosticEventBuffer(config))
 {
 }
@@ -642,4 +640,4 @@ TxEventManager::newFeeEvent(AccountID const& feeSource, int64_t amount)
     mTxEvents.emplace_back(std::move(ev));
 }
 
-} // namespace stellar
+} // namespace stellar
diff --git a/src/transactions/EventManager.h b/src/transactions/EventManager.h
@@ -104,13 +104,12 @@ class TxEventManager
     uint32_t mProtocolVersion;
     Hash const& mNetworkID;
     Config const& mConfig;
-    TransactionFrameBase const& mTx;
     xdr::xvector<ContractEvent> mTxEvents;
     DiagnosticEventBuffer mDiagnosticEvents;
 
   public:
     TxEventManager(uint32_t protocolVersion, Hash const& networkID,
-                   Config const& config, TransactionFrameBase const& tx);
+                   Config const& config);
 
     OpEventManager createNewOpEventManager(Memo const& memo);
 
@@ -137,4 +136,4 @@ class TxEventManager
 #endif
 };
 
-}
+}
diff --git a/src/transactions/test/FeeBumpTransactionTests.cpp b/src/transactions/test/FeeBumpTransactionTests.cpp
diff --git a/src/transactions/test/InvokeHostFunctionTests.cpp b/src/transactions/test/InvokeHostFunctionTests.cpp
@@ -574,7 +574,7 @@ TEST_CASE("basic contract invocation", "[tx][soroban]")
         auto timerBefore = hostFnExecTimer.count();
         auto txEventManager = TxEventManager(test.getLedgerVersion(),
                                              test.getApp().getNetworkID(),
-                                             test.getApp().getConfig(), *tx);
+                                             test.getApp().getConfig());
         bool success = tx->apply(test.getApp().getAppConnector(), rootLtx, txm,
                                  result, txEventManager);
         REQUIRE(hostFnExecTimer.count() - timerBefore > 0);
diff --git a/src/transactions/test/TransactionTestFrame.cpp b/src/transactions/test/TransactionTestFrame.cpp
@@ -48,8 +48,7 @@ TransactionTestFrame::apply(AppConnector& app, AbstractLedgerTxn& ltx,
                             Hash const& sorobanBasePrngSeed)
 {
     TxEventManager txEventManager(ltx.loadHeader().current().ledgerVersion,
-                                  app.getNetworkID(), app.getConfig(),
-                                  *mTransactionFrame);
+                                  app.getNetworkID(), app.getConfig());
     return mTransactionFrame->apply(app, ltx, meta, mTransactionTxResult,
                                     txEventManager, sorobanBasePrngSeed);
 }
@@ -119,13 +118,6 @@ TransactionTestFrame::checkValid(AppConnector& app, LedgerSnapshot const& ls,
     return mTransactionTxResult;
 }
 
-void
-TransactionTestFrame::processFeeSeqNum(AbstractLedgerTxn& ltx,
-                                       std::optional<int64_t> baseFee)
-{
-    mTransactionTxResult = mTransactionFrame->processFeeSeqNum(ltx, baseFee);
-}
-
 bool
 TransactionTestFrame::checkValidForTesting(AppConnector& app,
                                            AbstractLedgerTxn& ltxOuter,
@@ -344,4 +336,4 @@ TransactionTestFrame::XDRProvidesValidFee() const
 {
     return mTransactionFrame->XDRProvidesValidFee();
 }
-}
+}
diff --git a/src/transactions/test/TransactionTestFrame.h b/src/transactions/test/TransactionTestFrame.h
@@ -40,9 +40,6 @@ class TransactionTestFrame : public TransactionFrameBase
                               uint64_t lowerBoundCloseTimeOffset,
                               uint64_t upperBoundCloseTimeOffset);
 
-    void processFeeSeqNum(AbstractLedgerTxn& ltx,
-                          std::optional<int64_t> baseFee);
-
     void addSignature(SecretKey const& secretKey);
     void addSignature(DecoratedSignature const& signature);
 
@@ -142,4 +139,4 @@ class TransactionTestFrame : public TransactionFrameBase
         return true;
     }
 };
-}
+}
diff --git a/test-tx-meta-baseline-current/FeeBumpTransactionTests.json b/test-tx-meta-baseline-current/FeeBumpTransactionTests.json
@@ -91,7 +91,7 @@
 	"fee bump transactions|protocol version 12|validity|valid" : [ "xVjUo8vzo2Q=" ],
 	"fee bump transactions|protocol version 13|apply|bad signatures" : [ "yBA6fSUSbfk=", "edVLcYVg2uU=" ],
 	"fee bump transactions|protocol version 13|apply|extra signatures" : [ "Ai4B0vnogns=", "uentwW2oan8=", "MFJBMeCW+nE=" ],
-	"fee bump transactions|protocol version 13|apply|fee source does not exist" : [ "yBA6fSUSbfk=", "kBNyxXyMy3w=" ],
+	"fee bump transactions|protocol version 13|apply|fee source does not exist" : [ "yBA6fSUSbfk=", "6vF8WYsyny4=" ],
 	"fee bump transactions|protocol version 13|apply|inner transaction fails, operation level" : [ "yBA6fSUSbfk=" ],
 	"fee bump transactions|protocol version 13|apply|inner transaction fails, transaction level" : [ "yBA6fSUSbfk=", "9q0W0W8WRGY=" ],
 	"fee bump transactions|protocol version 13|apply|insufficient balance" : [ "yBA6fSUSbfk=", "eeqG9xT+CnE=" ],
@@ -107,7 +107,7 @@
 	"fee bump transactions|protocol version 13|validity|valid" : [ "xVjUo8vzo2Q=" ],
 	"fee bump transactions|protocol version 14|apply|bad signatures" : [ "yBA6fSUSbfk=", "edVLcYVg2uU=" ],
 	"fee bump transactions|protocol version 14|apply|extra signatures" : [ "Ai4B0vnogns=", "uentwW2oan8=", "MFJBMeCW+nE=" ],
-	"fee bump transactions|protocol version 14|apply|fee source does not exist" : [ "yBA6fSUSbfk=", "kBNyxXyMy3w=" ],
+	"fee bump transactions|protocol version 14|apply|fee source does not exist" : [ "yBA6fSUSbfk=", "6vF8WYsyny4=" ],
 	"fee bump transactions|protocol version 14|apply|inner transaction fails, operation level" : [ "yBA6fSUSbfk=" ],
 	"fee bump transactions|protocol version 14|apply|inner transaction fails, transaction level" : [ "yBA6fSUSbfk=", "9q0W0W8WRGY=" ],
 	"fee bump transactions|protocol version 14|apply|insufficient balance" : [ "yBA6fSUSbfk=", "eeqG9xT+CnE=" ],
@@ -124,7 +124,7 @@
 	"fee bump transactions|protocol version 14|validity|valid" : [ "xVjUo8vzo2Q=" ],
 	"fee bump transactions|protocol version 15|apply|bad signatures" : [ "yBA6fSUSbfk=", "edVLcYVg2uU=" ],
 	"fee bump transactions|protocol version 15|apply|extra signatures" : [ "Ai4B0vnogns=", "uentwW2oan8=", "MFJBMeCW+nE=" ],
-	"fee bump transactions|protocol version 15|apply|fee source does not exist" : [ "yBA6fSUSbfk=", "kBNyxXyMy3w=" ],
+	"fee bump transactions|protocol version 15|apply|fee source does not exist" : [ "yBA6fSUSbfk=", "6vF8WYsyny4=" ],
 	"fee bump transactions|protocol version 15|apply|inner transaction fails, operation level" : [ "yBA6fSUSbfk=" ],
 	"fee bump transactions|protocol version 15|apply|inner transaction fails, transaction level" : [ "yBA6fSUSbfk=", "9q0W0W8WRGY=" ],
 	"fee bump transactions|protocol version 15|apply|insufficient balance" : [ "yBA6fSUSbfk=", "eeqG9xT+CnE=" ],
@@ -141,7 +141,7 @@
 	"fee bump transactions|protocol version 15|validity|valid" : [ "xVjUo8vzo2Q=" ],
 	"fee bump transactions|protocol version 16|apply|bad signatures" : [ "yBA6fSUSbfk=", "edVLcYVg2uU=" ],
 	"fee bump transactions|protocol version 16|apply|extra signatures" : [ "Ai4B0vnogns=", "uentwW2oan8=", "MFJBMeCW+nE=" ],
-	"fee bump transactions|protocol version 16|apply|fee source does not exist" : [ "yBA6fSUSbfk=", "kBNyxXyMy3w=" ],
+	"fee bump transactions|protocol version 16|apply|fee source does not exist" : [ "yBA6fSUSbfk=", "6vF8WYsyny4=" ],
 	"fee bump transactions|protocol version 16|apply|inner transaction fails, operation level" : [ "yBA6fSUSbfk=" ],
 	"fee bump transactions|protocol version 16|apply|inner transaction fails, transaction level" : [ "yBA6fSUSbfk=", "9q0W0W8WRGY=" ],
 	"fee bump transactions|protocol version 16|apply|insufficient balance" : [ "yBA6fSUSbfk=", "eeqG9xT+CnE=" ],
@@ -158,7 +158,7 @@
 	"fee bump transactions|protocol version 16|validity|valid" : [ "xVjUo8vzo2Q=" ],
 	"fee bump transactions|protocol version 17|apply|bad signatures" : [ "yBA6fSUSbfk=", "edVLcYVg2uU=" ],
 	"fee bump transactions|protocol version 17|apply|extra signatures" : [ "Ai4B0vnogns=", "uentwW2oan8=", "MFJBMeCW+nE=" ],
-	"fee bump transactions|protocol version 17|apply|fee source does not exist" : [ "yBA6fSUSbfk=", "kBNyxXyMy3w=" ],
+	"fee bump transactions|protocol version 17|apply|fee source does not exist" : [ "yBA6fSUSbfk=", "6vF8WYsyny4=" ],
 	"fee bump transactions|protocol version 17|apply|inner transaction fails, operation level" : [ "yBA6fSUSbfk=" ],
 	"fee bump transactions|protocol version 17|apply|inner transaction fails, transaction level" : [ "yBA6fSUSbfk=", "9q0W0W8WRGY=" ],
 	"fee bump transactions|protocol version 17|apply|insufficient balance" : [ "yBA6fSUSbfk=", "eeqG9xT+CnE=" ],
@@ -175,7 +175,7 @@
 	"fee bump transactions|protocol version 17|validity|valid" : [ "xVjUo8vzo2Q=" ],
 	"fee bump transactions|protocol version 18|apply|bad signatures" : [ "yBA6fSUSbfk=", "edVLcYVg2uU=" ],
 	"fee bump transactions|protocol version 18|apply|extra signatures" : [ "Ai4B0vnogns=", "uentwW2oan8=", "MFJBMeCW+nE=" ],
-	"fee bump transactions|protocol version 18|apply|fee source does not exist" : [ "yBA6fSUSbfk=", "kBNyxXyMy3w=" ],
+	"fee bump transactions|protocol version 18|apply|fee source does not exist" : [ "yBA6fSUSbfk=", "6vF8WYsyny4=" ],
 	"fee bump transactions|protocol version 18|apply|inner transaction fails, operation level" : [ "yBA6fSUSbfk=" ],
 	"fee bump transactions|protocol version 18|apply|inner transaction fails, transaction level" : [ "yBA6fSUSbfk=", "9q0W0W8WRGY=" ],
 	"fee bump transactions|protocol version 18|apply|insufficient balance" : [ "yBA6fSUSbfk=", "eeqG9xT+CnE=" ],
@@ -192,7 +192,7 @@
 	"fee bump transactions|protocol version 18|validity|valid" : [ "xVjUo8vzo2Q=" ],
 	"fee bump transactions|protocol version 19|apply|bad signatures" : [ "liXKpc6omVU=", "E9++XFx985Q=" ],
 	"fee bump transactions|protocol version 19|apply|extra signatures" : [ "yMi8mNnA+z8=", "BJxa1AvCWhU=", "U9HtTBRBJRE=" ],
-	"fee bump transactions|protocol version 19|apply|fee source does not exist" : [ "liXKpc6omVU=", "I0h6USDr4vI=" ],
+	"fee bump transactions|protocol version 19|apply|fee source does not exist" : [ "liXKpc6omVU=", "cku6/VBG2Hs=" ],
 	"fee bump transactions|protocol version 19|apply|inner transaction fails, operation level" : [ "liXKpc6omVU=" ],
 	"fee bump transactions|protocol version 19|apply|inner transaction fails, transaction level" : [ "liXKpc6omVU=", "OqLmcePeqzU=" ],
 	"fee bump transactions|protocol version 19|apply|insufficient balance" : [ "liXKpc6omVU=", "0G4Li9bOzuE=" ],
@@ -224,7 +224,7 @@
 	"fee bump transactions|protocol version 1|validity|valid" : [ "9uN0oJ2mwX0=" ],
 	"fee bump transactions|protocol version 20|apply|bad signatures" : [ "Efz5uZa7U84=", "ofyE/F/i2I0=" ],
 	"fee bump transactions|protocol version 20|apply|extra signatures" : [ "eAy5C7YazX0=", "YY06OIIOp/w=", "+366qVuw7RI=" ],
-	"fee bump transactions|protocol version 20|apply|fee source does not exist" : [ "Efz5uZa7U84=", "UnwqqyolEMk=" ],
+	"fee bump transactions|protocol version 20|apply|fee source does not exist" : [ "Efz5uZa7U84=", "ythK0ALwS9g=" ],
 	"fee bump transactions|protocol version 20|apply|inner transaction fails, operation level" : [ "Efz5uZa7U84=" ],
 	"fee bump transactions|protocol version 20|apply|inner transaction fails, transaction level" : [ "Efz5uZa7U84=", "0kuWmO0K+zM=" ],
 	"fee bump transactions|protocol version 20|apply|insufficient balance" : [ "Efz5uZa7U84=", "jtss133AcPA=" ],
@@ -241,7 +241,7 @@
 	"fee bump transactions|protocol version 20|validity|valid" : [ "g4SMgLdHmkY=" ],
 	"fee bump transactions|protocol version 21|apply|bad signatures" : [ "Efz5uZa7U84=", "ofyE/F/i2I0=" ],
 	"fee bump transactions|protocol version 21|apply|extra signatures" : [ "eAy5C7YazX0=", "YY06OIIOp/w=", "+366qVuw7RI=" ],
-	"fee bump transactions|protocol version 21|apply|fee source does not exist" : [ "Efz5uZa7U84=", "UnwqqyolEMk=" ],
+	"fee bump transactions|protocol version 21|apply|fee source does not exist" : [ "Efz5uZa7U84=", "ythK0ALwS9g=" ],
 	"fee bump transactions|protocol version 21|apply|inner transaction fails, operation level" : [ "Efz5uZa7U84=" ],
 	"fee bump transactions|protocol version 21|apply|inner transaction fails, transaction level" : [ "Efz5uZa7U84=", "0kuWmO0K+zM=" ],
 	"fee bump transactions|protocol version 21|apply|insufficient balance" : [ "Efz5uZa7U84=", "jtss133AcPA=" ],
@@ -258,7 +258,7 @@
 	"fee bump transactions|protocol version 21|validity|valid" : [ "g4SMgLdHmkY=" ],
 	"fee bump transactions|protocol version 22|apply|bad signatures" : [ "Efz5uZa7U84=", "ofyE/F/i2I0=" ],
 	"fee bump transactions|protocol version 22|apply|extra signatures" : [ "eAy5C7YazX0=", "YY06OIIOp/w=", "+366qVuw7RI=" ],
-	"fee bump transactions|protocol version 22|apply|fee source does not exist" : [ "Efz5uZa7U84=", "UnwqqyolEMk=" ],
+	"fee bump transactions|protocol version 22|apply|fee source does not exist" : [ "Efz5uZa7U84=", "ythK0ALwS9g=" ],
 	"fee bump transactions|protocol version 22|apply|inner transaction fails, operation level" : [ "Efz5uZa7U84=" ],
 	"fee bump transactions|protocol version 22|apply|inner transaction fails, transaction level" : [ "Efz5uZa7U84=", "0kuWmO0K+zM=" ],
 	"fee bump transactions|protocol version 22|apply|insufficient balance" : [ "Efz5uZa7U84=", "jtss133AcPA=" ],
diff --git a/test-tx-meta-baseline-next/FeeBumpTransactionTests.json b/test-tx-meta-baseline-next/FeeBumpTransactionTests.json

Original file line number	Diff line number	Diff line change
`@@ -556,12 +556,10 @@ OpEventManager::flushContractEvents(xdr::xvector<ContractEvent>& buf)`
`556`	`556`	`};`
`557`	`557`
`558`	`558`	`TxEventManager::TxEventManager(uint32_t protocolVersion, Hash const& networkID,`
`559`		`- Config const& config,`
`560`		`- TransactionFrameBase const& tx)`
	`559`	`+ Config const& config)`
`561`	`560`	`: mProtocolVersion(protocolVersion)`
`562`	`561`	`, mNetworkID(networkID)`
`563`	`562`	`, mConfig(config)`
`564`		`- , mTx(tx)`
`565`	`563`	`, mDiagnosticEvents(DiagnosticEventBuffer(config))`
`566`	`564`	`{`
`567`	`565`	`}`
`@@ -642,4 +640,4 @@ TxEventManager::newFeeEvent(AccountID const& feeSource, int64_t amount)`
`642`	`640`	`mTxEvents.emplace_back(std::move(ev));`
`643`	`641`	`}`
`644`	`642`
`645`		`-} // namespace stellar`
	`643`	`+} // namespace stellar`
Original file line number	Diff line number	Diff line change
`@@ -48,8 +48,7 @@ TransactionTestFrame::apply(AppConnector& app, AbstractLedgerTxn& ltx,`
`48`	`48`	`Hash const& sorobanBasePrngSeed)`
`49`	`49`	`{`
`50`	`50`	`TxEventManager txEventManager(ltx.loadHeader().current().ledgerVersion,`
`51`		`- app.getNetworkID(), app.getConfig(),`
`52`		`- *mTransactionFrame);`
	`51`	`+ app.getNetworkID(), app.getConfig());`
`53`	`52`	`return mTransactionFrame->apply(app, ltx, meta, mTransactionTxResult,`
`54`	`53`	`txEventManager, sorobanBasePrngSeed);`
`55`	`54`	`}`
`@@ -119,13 +118,6 @@ TransactionTestFrame::checkValid(AppConnector& app, LedgerSnapshot const& ls,`
`119`	`118`	`return mTransactionTxResult;`
`120`	`119`	`}`
`121`	`120`
`122`		`-void`
`123`		`-TransactionTestFrame::processFeeSeqNum(AbstractLedgerTxn& ltx,`
`124`		`- std::optional<int64_t> baseFee)`
`125`		`-{`
`126`		`- mTransactionTxResult = mTransactionFrame->processFeeSeqNum(ltx, baseFee);`
`127`		`-}`
`128`		`-`
`129`	`121`	`bool`
`130`	`122`	`TransactionTestFrame::checkValidForTesting(AppConnector& app,`
`131`	`123`	`AbstractLedgerTxn& ltxOuter,`
`@@ -344,4 +336,4 @@ TransactionTestFrame::XDRProvidesValidFee() const`
`344`	`336`	`{`
`345`	`337`	`return mTransactionFrame->XDRProvidesValidFee();`
`346`	`338`	`}`
`347`		`-}`
	`339`	`+}`