refactor(e2e): use strict event typing #3079
Conversation
gartnera
added
no-changelog
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 Walkthrough📝 WalkthroughWalkthroughThis pull request introduces several enhancements to end-to-end (E2E) testing and transaction handling within the codebase. Key changes include the refinement of event handling for ERC20-related tests, improving the reliability and clarity of event retrieval processes. It also updates the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
gartnera
force-pushed
the
e2e-typed-events
branch
2 times, most recently
from
65cf43a to
9c95e2c
Compare
gartnera
requested review from
brewmaster012,
kingpinXD,
lumtis,
ws4charlie,
skosito,
swift1337 and
fbac
as code owners
gartnera
force-pushed
the
e2e-typed-events
branch
from
1c2f82d to
e97c3bf
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (8)
e2e/e2etests/test_migrate_erc20_custody_funds.go (2)
38-39: Enhance error message clarity in event validation.The error message could be more descriptive to help with debugging failures.
Consider this improvement:
-require.True(r, ok, "no EventERC20CustodyFundsMigration in %s", res.TxHash) +require.True(r, ok, "expected EventERC20CustodyFundsMigration event not found in transaction %s", res.TxHash)
Line range hint
53-54: Consider refactoring the artificial state modification.The comment indicates that the ERC20 custody address is being modified to prevent accounting check failures. This might mask potential issues and makes the test less reliable.
Consider:
- Adding validation before the modification to ensure the migration was successful
- Creating a separate test case for accounting validation
- Documenting why this modification is necessary in more detail
e2e/e2etests/test_pause_erc20_custody.go (2)
35-38: LGTM! Consider enhancing the error message.The event handling implementation is type-safe and clear. The error message could be more descriptive to aid debugging.
-require.True(r, ok, "no EventERC20CustodyPausing in %s", res.TxHash) +require.True(r, ok, "expected EventERC20CustodyPausing event not found in transaction %s", res.TxHash)
40-47: LGTM! Consider enhancing error handling.The CCTX handling logic is sound, with appropriate error checking and logging.
-require.NoError(r, err) +require.NoError(r, err, "failed to fetch CCTX with index %s", event.CctxIndex)e2e/e2etests/test_whitelist_erc20.go (1)
Line range hint
13-142: Consider breaking down the test into smaller, focused sub-tests.The test function handles multiple concerns (deployment, whitelisting, verification, deposits, withdrawals). Consider refactoring into smaller sub-tests using
t.Run()for better maintainability and clearer failure isolation.Example structure:
func TestWhitelistERC20(r *runner.E2ERunner, _ []string) { // Common setup erc20Addr, cleanup := setupERC20(r) defer cleanup() t.Run("Whitelist", func(t *testing.T) { // Whitelisting specific tests }) t.Run("Deposit", func(t *testing.T) { // Deposit specific tests }) t.Run("Withdraw", func(t *testing.T) { // Withdraw specific tests }) }This structure would:
- Improve test failure isolation
- Make the test flow easier to understand
- Allow for parallel test execution if needed
- Facilitate adding new test cases
e2e/runner/v2_migration.go (2)
152-154: LGTM! Consider enhancing the error message.The transition to strictly typed events improves type safety and refactoring confidence. The error message could be more descriptive to aid debugging.
- require.True(r, ok, "no EventERC20CustodyFundsMigration in %s", res.TxHash) + require.True(r, ok, "failed to find EventERC20CustodyFundsMigration during ERC20 custody pause operation (tx: %s)", res.TxHash)
191-193: LGTM! Consider extracting the event handling logic.The event handling logic is duplicated from the previous segment. Consider extracting this into a helper method to maintain DRY principles and ensure consistent error handling.
func (r *E2ERunner) extractMigrationEventCCTXIndex(res *sdk.TxResponse, operation string) string { migrationEvent, ok := txserver.EventOfType[*crosschaintypes.EventERC20CustodyFundsMigration](res.Events) require.True(r, ok, "failed to find EventERC20CustodyFundsMigration during %s operation (tx: %s)", operation, res.TxHash) return migrationEvent.CctxIndex }Usage:
- migrationEvent, ok := txserver.EventOfType[*crosschaintypes.EventERC20CustodyFundsMigration](res.Events) - require.True(r, ok, "no EventERC20CustodyFundsMigration in %s", res.TxHash) - cctxIndex = migrationEvent.CctxIndex + cctxIndex = r.extractMigrationEventCCTXIndex(res, "ERC20 custody migration")e2e/e2etests/test_migrate_chain_support.go (1)
170-173: LGTM! Clean transition to typed event handling.The implementation correctly transitions from direct attribute fetching to using strictly typed events, which aligns with the PR objectives. The error handling is robust with a descriptive message that includes the transaction hash for debugging.
Consider enhancing the error message to be more specific about what went wrong:
- require.True(r, ok, "no EventERC20Whitelist in %s", res.TxHash) + require.True(r, ok, "failed to extract EventERC20Whitelist from transaction %s", res.TxHash)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (6)
e2e/e2etests/test_migrate_chain_support.go(1 hunks)e2e/e2etests/test_migrate_erc20_custody_funds.go(1 hunks)e2e/e2etests/test_pause_erc20_custody.go(2 hunks)e2e/e2etests/test_whitelist_erc20.go(1 hunks)e2e/runner/v2_migration.go(2 hunks)e2e/txserver/zeta_tx_server.go(4 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
e2e/e2etests/test_migrate_chain_support.go (1)
Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
e2e/e2etests/test_migrate_erc20_custody_funds.go (1)
Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
e2e/e2etests/test_pause_erc20_custody.go (1)
Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
e2e/e2etests/test_whitelist_erc20.go (1)
Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
e2e/runner/v2_migration.go (1)
Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
e2e/txserver/zeta_tx_server.go (1)
Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
🔇 Additional comments (10)
e2e/e2etests/test_migrate_erc20_custody_funds.go (1)
41-48: LGTM: Clean event-driven implementation.
The transition to using event.CctxIndex for CCTX querying and mining wait is well-implemented and aligns with the PR's objective of strict event typing.
e2e/e2etests/test_whitelist_erc20.go (1)
46-49: LGTM! Consider enhancing error messaging.
The transition to structured event handling using EventOfType is a solid improvement that aligns well with type safety goals.
Consider enhancing the error message to be more descriptive:
-require.True(r, ok, "no EventERC20Whitelist in %s", res.TxHash)
+require.True(r, ok, "expected EventERC20Whitelist not found in transaction %s response", res.TxHash)Let's verify consistent event usage across the codebase:
✅ Verification successful
The previous search didn't show the complete picture. We need to verify:
- The consistent usage of EventOfType across test files
- Any remaining direct attribute fetching in test files specifically
This will help us confirm if the transition to structured events is complete and consistent.
Consistent event handling pattern confirmed across the codebase
The verification shows that:
- The
EventOfTypepattern is consistently used across all test files and related code - No legacy
FetchAttributeFromTxResponseusage was found in test files - The implementation in
test_whitelist_erc20.gofollows the same pattern as other similar event handling cases (e.g., intest_migrate_chain_support.go)
The original suggestion to improve the error message remains valid but is a minor enhancement to an already well-structured implementation.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for consistent usage of EventOfType and direct attribute fetching
# Test 1: Look for any remaining direct attribute fetching
rg "FetchAttributeFromTxResponse.*whitelist_cctx_index|zrc20_address"
# Test 2: Verify consistent EventERC20Whitelist usage
ast-grep --pattern 'EventOfType[*crosschaintypes.EventERC20Whitelist]'
Length of output: 4001
Script:
#!/bin/bash
# Let's search for all EventOfType usage patterns to ensure consistent event handling
rg "EventOfType\[.*\]" -A 2
# Also check for any remaining direct attribute fetching specifically in test files
rg "FetchAttributeFromTxResponse" --type go --glob "*test*.go"
Length of output: 2721
e2e/e2etests/test_migrate_chain_support.go (1)
172-173: 🛠️ Refactor suggestion
Consider adding validations for extracted event data.
While the event extraction is correct, consider adding assertions to validate the extracted data before usage:
Add these validations after the event extraction:
erc20zrc20Addr := event.Zrc20Address
whitelistCCTXIndex := event.WhitelistCctxIndex
+require.NotEmpty(r, erc20zrc20Addr, "zrc20 address should not be empty")
+require.Greater(r, whitelistCCTXIndex, uint64(0), "whitelist CCTX index should be positive")e2e/txserver/zeta_tx_server.go (7)
35-35: Approved Addition of gogoproto/proto Import
The import of "github.com/cosmos/gogoproto/proto" is necessary for utilizing proto.Message in the generic functions. This enhances type safety in event handling.
361-363: Improved Event Handling with EventOfType
The use of EventOfType[*fungibletypes.EventSystemContractsDeployed] simplifies the retrieval of the EventSystemContractsDeployed event. This approach reduces complexity by avoiding manual attribute parsing.
369-369: Correct Construction of MsgUpdateSystemContract
Passing deployedEvent.SystemContract directly into NewMsgUpdateSystemContract ensures accurate updating of system contracts using the deployed event data.
376-379: Accurate Assignment of Deployed Contract Addresses
Assigning the addresses from deployedEvent to the SystemContractAddresses struct improves clarity and maintainability.
490-493: Efficient Retrieval of EventZRC20Deployed Events
Using EventsOfType[*fungibletypes.EventZRC20Deployed] streamlines the collection of deployed ZRC20 events, enhancing the readability and reliability of the code.
664-677: Effective Implementation of Generic Function EventsOfType
The generic function EventsOfType[T proto.Message] efficiently filters events of type T. This leverages Go generics to enhance code reusability and type safety in event processing.
679-691: EventOfType Simplifies Single Event Retrieval
The function EventOfType[T proto.Message] provides a clean and concise way to retrieve a single event of a specific type. This promotes code clarity and reduces boilerplate in event handling.
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments.
Actionable comments posted: 7
🧹 Outside diff range and nitpick comments (11)
e2e/e2etests/test_migrate_erc20_custody_funds.go (1)
Line range hint
53-54: Consider redesigning the final state validation.The need to artificially set
ERC20CustodyAddrto prevent accounting check failures suggests a potential design issue. This approach could mask real problems and makes the test less reliable.Consider either:
- Implementing proper state tracking that doesn't require artificial manipulation
- Adding explicit validation of both old and new custody addresses' states
- Refactoring the accounting check to handle custody address changes gracefully
e2e/e2etests/test_pause_erc20_custody.go (1)
35-38: LGTM! Consider enhancing the error message.The event handling implementation is clean and type-safe, aligning well with the PR's objective of strict event typing.
Consider making the error message more descriptive:
-require.True(r, ok, "no EventERC20CustodyPausing in %s", res.TxHash) +require.True(r, ok, "expected EventERC20CustodyPausing event not found in transaction %s", res.TxHash)e2e/e2etests/test_whitelist_erc20.go (1)
46-49: LGTM! Consider enhancing the error message.The transition to type-safe event extraction using
EventOfTypeis a solid improvement. It provides better compile-time safety and makes the code more maintainable during refactors.Consider making the error message more descriptive:
-require.True(r, ok, "no EventERC20Whitelist in %s", res.TxHash) +require.True(r, ok, "expected EventERC20Whitelist event not found in transaction %s", res.TxHash)e2e/e2etests/test_migrate_chain_support.go (2)
170-173: LGTM! Consider enhancing the error message.The transition to strictly typed event handling using generics is a solid improvement. The code properly checks for event presence and includes error context.
Consider making the error message more descriptive:
- require.True(r, ok, "no EventERC20Whitelist in %s", res.TxHash) + require.True(r, ok, "failed to extract EventERC20Whitelist from transaction %s", res.TxHash)
Line range hint
1-400: Consider architectural improvements to enhance test maintainability.The test implementation could benefit from the following improvements:
- Extract hardcoded values (e.g.,
EVM2RPCURL, chain IDs, amounts) into a configuration struct or constants file.- Break down the large test into smaller sub-tests using
t.Run()for better isolation and readability.- Add detailed comments explaining the purpose and expected outcome of each test phase.
Would you like assistance in implementing these improvements?
cmd/zetae2e/local/local.go (2)
188-191: Enhance comment clarity for setup steps.The implementation looks good, but the comment could be more descriptive about the emissions pool funding step.
Consider updating the comment to be more specific:
- // run setup steps that do not require tss + // run pre-TSS setup steps (emissions pool funding)
Line range hint
76-76: Consider architectural improvements for better maintainability.The implementation would benefit from the following improvements:
- Address the TODO comment by splitting the test setup logic into separate files
- Extract the emissions pool funding into a dedicated setup function
- Consider using more idiomatic Go error handling patterns
Example refactor for the emissions pool setup:
// setupEmissionsPool handles the emissions pool funding setup func (r *Runner) setupEmissionsPool(skipSetup bool) error { if skipSetup { return nil } return r.FundEmissionsPool() }This would make the main function more concise and easier to maintain:
- // run pre-TSS setup steps (emissions pool funding) - if !skipSetup { - noError(deployerRunner.FundEmissionsPool()) - } + noError(deployerRunner.setupEmissionsPool(skipSetup))Also applies to: 188-191
e2e/txserver/zeta_tx_server.go (4)
Line range hint
196-218: Validate non-empty message slice inBroadcastTx.The
BroadcastTxfunction now accepts a variadicmsgs ...sdktypes.Msgparameter. To prevent unintended behavior when no messages are provided (e.g., zero gas and fees), ensure thatmsgsis not empty before proceeding.Apply this diff to add a validation check:
func (zts ZetaTxServer) BroadcastTx(account string, msgs ...sdktypes.Msg) (*sdktypes.TxResponse, error) { + if len(msgs) == 0 { + return nil, errors.New("no messages provided to BroadcastTx") + } // Retrieve account number and sequence ... }
421-483: Review dependency on external packagegithub.com/samber/lo.The use of
lo.FilterMapenhances code readability but introduces an external dependency. Evaluate if this dependency aligns with project policies and whether standard library alternatives (e.g., traditional loops) could suffice to reduce dependency overhead.If acceptable, proceed; otherwise, refactor using standard constructs:
-deployMsgsIface := lo.FilterMap( - deployMsgs, - func(msg *fungibletypes.MsgDeployFungibleCoinZRC20, _ int) (sdktypes.Msg, bool) { - if skipChain(msg.ForeignChainId) { - return nil, false - } - return msg, true - }, -) +var deployMsgsIface []sdktypes.Msg +for _, msg := range deployMsgs { + if !skipChain(msg.ForeignChainId) { + deployMsgsIface = append(deployMsgsIface, msg) + } +}
664-677: Enhance error handling inEventsOfType.Currently, parsing errors are silently ignored. Logging or aggregating these errors can aid in diagnosing issues with event parsing, especially when expected events are not found.
Consider collecting errors:
func EventsOfType[T proto.Message](events []abci.Event) ([]T, bool) { var filteredEvents []T + var parseErrors []error for _, ev := range events { pEvent, err := sdktypes.ParseTypedEvent(ev) if err != nil { + parseErrors = append(parseErrors, err) continue } if typedEvent, ok := pEvent.(T); ok { filteredEvents = append(filteredEvents, typedEvent) } } + if len(filteredEvents) == 0 && len(parseErrors) > 0 { + // Optionally log or handle parseErrors + } return filteredEvents, len(filteredEvents) > 0 }
679-691: Consistent use of singular and plural in function names.The function
EventOfTypefetches a single event, whileEventsOfTypefetches multiple events. Ensure that this naming convention is consistently applied throughout the codebase for clarity.Confirm that all usages align with this convention, and consider adding documentation comments to clarify the purpose of each function.
🛑 Comments failed to post (7)
e2e/e2etests/test_migrate_erc20_custody_funds.go (1)
41-41: 🛠️ Refactor suggestion
Consider adding context timeout for CCTX query.
While the implementation is correct, adding a timeout would prevent potential test hangs during network issues.
- cctxRes, err := r.CctxClient.Cctx(r.Ctx, &crosschaintypes.QueryGetCctxRequest{Index: event.CctxIndex}) + ctx, cancel := context.WithTimeout(r.Ctx, 30*time.Second) + defer cancel() + cctxRes, err := r.CctxClient.Cctx(ctx, &crosschaintypes.QueryGetCctxRequest{Index: event.CctxIndex})📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.ctx, cancel := context.WithTimeout(r.Ctx, 30*time.Second) defer cancel() cctxRes, err := r.CctxClient.Cctx(ctx, &crosschaintypes.QueryGetCctxRequest{Index: event.CctxIndex})e2e/e2etests/test_pause_erc20_custody.go (2)
65-68: 🛠️ Refactor suggestion
Consider extracting a helper function to reduce duplication.
The event handling logic is duplicated between pause and unpause operations. Consider extracting a helper function to improve maintainability.
func assertERC20CustodyPausingEvent(t *testing.T, events []abci.Event, expectedPauseStatus bool, txHash string) *crosschaintypes.EventERC20CustodyPausing { event, ok := txserver.EventOfType[*crosschaintypes.EventERC20CustodyPausing](events) require.True(t, ok, "expected EventERC20CustodyPausing event not found in transaction %s", txHash) require.Equal(t, expectedPauseStatus, event.Pause, "unexpected pause status") return event }Usage:
-event, ok = txserver.EventOfType[*crosschaintypes.EventERC20CustodyPausing](res.Events) -require.True(r, ok, "no EventERC20CustodyPausing in %s", res.TxHash) -require.False(r, event.Pause, "should be unpaused") +event = assertERC20CustodyPausingEvent(r.T, res.Events, false, res.TxHash)
70-70: 🛠️ Refactor suggestion
Consider extending the helper function to include CCTX handling.
The CCTX handling logic is also duplicated. Consider extending the helper function to include CCTX operations.
func handleERC20CustodyPausingEvent( t *testing.T, r *runner.E2ERunner, events []abci.Event, expectedPauseStatus bool, txHash string, operation string, ) *crosschaintypes.CrossChainTx { event := assertERC20CustodyPausingEvent(t, events, expectedPauseStatus, txHash) cctxRes, err := r.CctxClient.Cctx(r.Ctx, &crosschaintypes.QueryGetCctxRequest{Index: event.CctxIndex}) require.NoError(t, err) cctx := cctxRes.CrossChainTx r.Logger.CCTX(*cctx, operation) r.WaitForMinedCCTXFromIndex(event.CctxIndex) return cctx }Usage:
-cctxRes, err = r.CctxClient.Cctx(r.Ctx, &crosschaintypes.QueryGetCctxRequest{Index: event.CctxIndex}) -require.NoError(r, err) - -cctx = cctxRes.CrossChainTx -r.Logger.CCTX(*cctx, "unpausing") - -r.WaitForMinedCCTXFromIndex(event.CctxIndex) +cctx = handleERC20CustodyPausingEvent(r.T, r, res.Events, false, res.TxHash, "unpausing")Also applies to: 77-77
e2e/runner/v2_migration.go (1)
191-193: 🛠️ Refactor suggestion
Consider extracting event handling into a helper function.
This event handling pattern is duplicated. Consider creating a helper function to reduce duplication and improve maintainability:
func (r *E2ERunner) getMigrationEventCctxIndex(res *sdk.TxResponse) string { migrationEvent, ok := txserver.EventOfType[*crosschaintypes.EventERC20CustodyFundsMigration](res.Events) require.True(r, ok, "no EventERC20CustodyFundsMigration in %s", res.TxHash) return migrationEvent.CctxIndex }Then use it in both locations:
cctxIndex := r.getMigrationEventCctxIndex(res)e2e/txserver/zeta_tx_server.go (3)
560-573:
⚠️ Potential issueAvoid using
Mustmethods that may cause panics.In
InitializeLiquidityCaps, usingMustGetAccountAddressFromNamecan lead to panics if the account name is not found. Replace it withGetAccountAddressFromNameand handle the error to improve robustness.Apply this diff to handle errors gracefully:
func (zts ZetaTxServer) InitializeLiquidityCaps(zrc20s ...string) error { liquidityCap := sdktypes.NewUint(1e18).MulUint64(1e12) msgs := make([]sdktypes.Msg, len(zrc20s)) + accountAddr, err := zts.GetAccountAddressFromName(utils.OperationalPolicyName) + if err != nil { + return fmt.Errorf("failed to get account address: %w", err) + } for i, zrc20 := range zrc20s { msgs[i] = fungibletypes.NewMsgUpdateZRC20LiquidityCap( - zts.MustGetAccountAddressFromName(utils.OperationalPolicyName), + accountAddr, zrc20, liquidityCap, ) } _, err := zts.BroadcastTx(utils.OperationalPolicyName, msgs...) return err }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.// InitializeLiquidityCaps initializes the liquidity cap for the given coin with a large value func (zts ZetaTxServer) InitializeLiquidityCaps(zrc20s ...string) error { liquidityCap := sdktypes.NewUint(1e18).MulUint64(1e12) msgs := make([]sdktypes.Msg, len(zrc20s)) accountAddr, err := zts.GetAccountAddressFromName(utils.OperationalPolicyName) if err != nil { return fmt.Errorf("failed to get account address: %w", err) } for i, zrc20 := range zrc20s { msgs[i] = fungibletypes.NewMsgUpdateZRC20LiquidityCap( accountAddr, zrc20, liquidityCap, ) } _, err = zts.BroadcastTx(utils.OperationalPolicyName, msgs...) return err
361-363: 🛠️ Refactor suggestion
Augment error message when event is not found.
In
DeploySystemContracts, if theEventSystemContractsDeployedis not present, consider providing additional context in the error message to facilitate debugging.Apply this diff to enhance the error message:
if !ok { - return SystemContractAddresses{}, fmt.Errorf("no EventSystemContractsDeployed in %s", res.TxHash) + return SystemContractAddresses{}, fmt.Errorf("EventSystemContractsDeployed not found in transaction events for TxHash: %s", res.TxHash) }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.deployedEvent, ok := EventOfType[*fungibletypes.EventSystemContractsDeployed](res.Events) if !ok { return SystemContractAddresses{}, fmt.Errorf("EventSystemContractsDeployed not found in transaction events for TxHash: %s", res.TxHash)
217-218: 🛠️ Refactor suggestion
Adjust gas and fee calculations for accuracy.
Multiplying gas and fees by the number of messages may not account for the individual gas requirements of each message, especially if they vary significantly. Consider calculating the total gas and fees based on the sum of estimated gas for each message to ensure accurate fee allocation.
Refactor the gas and fee calculation:
// Initialize totalGas and totalFees +totalGas := uint64(0) +totalFees := sdktypes.NewCoins() for _, msg := range msgs { + // Estimate gas for each message + gas, err := zts.txFactory.WithGas(0).Simulate(txBuilder.SetMsgs(msg)) + if err != nil { + return nil, err + } + totalGas += gas +} // Set total gas and fees -txBuilder.SetGasLimit(zts.txFactory.Gas() * uint64(len(msgs))) +txBuilder.SetGasLimit(totalGas) -txBuilder.SetFeeAmount(zts.txFactory.Fees().MulInt(sdktypes.NewInt(int64(len(msgs))))) +txBuilder.SetFeeAmount(sdktypes.NewCoins(sdktypes.NewCoin("azeta", sdktypes.NewInt(int64(totalGas)*gasPrice))))Ensure
gasPriceis defined appropriately.Committable suggestion skipped: line range outside the PR's diff.
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
Description
Always use strict event typing in e2e tests. This improves confidence during refactors and enables easily finding where the event is emitted:
Screen.Recording.2024-10-31.at.2.10.36.PM.mov
Based on #3077 since I started using strictly typed events in that PR
How Has This Been Tested?
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Documentation
Refactor