feat(ton): send real gas usage #3699
Conversation
|
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 📝 WalkthroughWalkthroughThis pull request adds a new changelog entry under version v29.0.0 for using real gas usage in TON withdrawals. It removes obsolete commented code from the Sui client, modifies the TON observer’s deposit logic by replacing a hardcoded gas limit with a constant value, augments the Observer with an atomic field and helper methods for gas price management, and refactors outbound voting functions by updating type aliases, consolidating parameters, and improving error handling. Changes
Sequence Diagram(s)sequenceDiagram
participant Block as Block Data
participant Observer
participant VoteService as VoteGasPrice Service
Block->>Observer: Provide block info (blockID.Seqno)
Observer->>Observer: Compute new gas price
Observer->>Observer: setLatestGasPrice(newGasPrice)
Observer->>VoteService: PostVoteGasPrice(newGasPrice, block number)
VoteService-->>Observer: Acknowledgement
Observer-->>Block: Return success
sequenceDiagram
participant Client
participant Observer
participant OutboundService as External Withdrawal Service
Client->>Observer: VoteOutboundIfConfirmed(ctx, cctypes.CrossChainTx)
Observer->>OutboundService: postVoteOutbound(ctx, cctx, outboundRes, withdrawal)
OutboundService-->>Observer: Transaction response
Observer-->>Client: Return vote status
Possibly related PRs
Suggested labels
Suggested reviewers
🪧 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
|
|
!!!WARNING!!! Be very careful about using Only suppress a single rule (or a specific set of rules) within a section of code, while continuing to scan for other problems. To do this, you can list the rule(s) to be suppressed within the #nosec annotation, e.g: /* #nosec G401 */ or //#nosec G201 G202 G203 Pay extra attention to the way |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #3699 +/- ##
===========================================
- Coverage 64.58% 64.58% -0.01%
===========================================
Files 469 469
Lines 32915 32918 +3
===========================================
Hits 21259 21259
- Misses 10691 10694 +3
Partials 965 965
🚀 New features to boost your workflow:
|
![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
🧹 Nitpick comments (1)
zetaclient/chains/ton/observer/outbound.go (1)
227-236: Enhanced gas price retrieval and validationThe implementation now retrieves the gas price atomically and includes proper validation before proceeding. The conversion to math.Int is also handled correctly.
Consider adding a fallback mechanism for when the gas price is not set, such as triggering a gas price update rather than returning an error.
if !ok { - return errors.New("gas price is not set (call PostGasPrice first)") + if err := ob.PostGasPrice(ctx); err != nil { + return errors.Wrap(err, "failed to update gas price") + } + gasPrice, ok = ob.getLatestGasPrice() + if !ok { + return errors.New("gas price is still not set after update attempt") + } }🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 227-232: zetaclient/chains/ton/observer/outbound.go#L227-L232
Added lines #L227 - L232 were not covered by tests
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
changelog.md(1 hunks)zetaclient/chains/sui/sui.go(0 hunks)zetaclient/chains/ton/observer/inbound.go(1 hunks)zetaclient/chains/ton/observer/observer.go(4 hunks)zetaclient/chains/ton/observer/outbound.go(4 hunks)
💤 Files with no reviewable changes (1)
- zetaclient/chains/sui/sui.go
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.go`: Review the Go code, point out issues relative to ...
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
zetaclient/chains/ton/observer/inbound.gozetaclient/chains/ton/observer/observer.gozetaclient/chains/ton/observer/outbound.go
🪛 GitHub Check: codecov/patch
zetaclient/chains/ton/observer/observer.go
[warning] 89-90: zetaclient/chains/ton/observer/observer.go#L89-L90
Added lines #L89 - L90 were not covered by tests
[warning] 94-97: zetaclient/chains/ton/observer/observer.go#L94-L97
Added lines #L94 - L97 were not covered by tests
[warning] 99-99: zetaclient/chains/ton/observer/observer.go#L99
Added line #L99 was not covered by tests
[warning] 101-101: zetaclient/chains/ton/observer/observer.go#L101
Added line #L101 was not covered by tests
[warning] 135-138: zetaclient/chains/ton/observer/observer.go#L135-L138
Added lines #L135 - L138 were not covered by tests
[warning] 141-142: zetaclient/chains/ton/observer/observer.go#L141-L142
Added lines #L141 - L142 were not covered by tests
zetaclient/chains/ton/observer/outbound.go
[warning] 31-31: zetaclient/chains/ton/observer/outbound.go#L31
Added line #L31 was not covered by tests
[warning] 47-47: zetaclient/chains/ton/observer/outbound.go#L47
Added line #L47 was not covered by tests
[warning] 215-217: zetaclient/chains/ton/observer/outbound.go#L215-L217
Added lines #L215 - L217 were not covered by tests
[warning] 221-221: zetaclient/chains/ton/observer/outbound.go#L221
Added line #L221 was not covered by tests
[warning] 227-232: zetaclient/chains/ton/observer/outbound.go#L227-L232
Added lines #L227 - L232 were not covered by tests
[warning] 235-237: zetaclient/chains/ton/observer/outbound.go#L235-L237
Added lines #L235 - L237 were not covered by tests
[warning] 241-244: zetaclient/chains/ton/observer/outbound.go#L241-L244
Added lines #L241 - L244 were not covered by tests
[warning] 246-246: zetaclient/chains/ton/observer/outbound.go#L246
Added line #L246 was not covered by tests
[warning] 250-250: zetaclient/chains/ton/observer/outbound.go#L250
Added line #L250 was not covered by tests
[warning] 253-253: zetaclient/chains/ton/observer/outbound.go#L253
Added line #L253 was not covered by tests
[warning] 257-257: zetaclient/chains/ton/observer/outbound.go#L257
Added line #L257 was not covered by tests
[warning] 261-262: zetaclient/chains/ton/observer/outbound.go#L261-L262
Added lines #L261 - L262 were not covered by tests
[warning] 267-271: zetaclient/chains/ton/observer/outbound.go#L267-L271
Added lines #L267 - L271 were not covered by tests
🔇 Additional comments (7)
changelog.md (1)
14-14: New feature entry looks goodThe changelog entry properly documents the addition of real gas usage for TON withdrawals, linked to the correct pull request number.
zetaclient/chains/ton/observer/inbound.go (1)
257-257: Replacing hardcoded gas limit with constant valueGood change replacing the hardcoded value of 0 with
maxGasLimitfor better consistency and maintainability. This aligns with the PR objective of using real gas usage for TON withdrawals.zetaclient/chains/ton/observer/observer.go (1)
28-28: Good addition of atomic gas price trackingAdding an atomic field for tracking the latest gas price ensures thread-safe operations across concurrent accesses.
zetaclient/chains/ton/observer/outbound.go (4)
20-22: Clear documentation of maximum gas limitGood addition of comments explaining the source of the maximum gas limit value and its potential for future changes.
241-244: Dynamic gas parameters in vote outbound messageNow using actual block height, gas used, gas price, and maximum gas limit values instead of hardcoded ones, which improves accuracy and maintainability.
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 241-244: zetaclient/chains/ton/observer/outbound.go#L241-L244
Added lines #L241 - L244 were not covered by tests
267-271: Improved error handling with switch statementThe error handling now uses a clearer switch statement pattern, improving readability and maintainability.
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 267-271: zetaclient/chains/ton/observer/outbound.go#L267-L271
Added lines #L267 - L271 were not covered by tests
13-13: Updated imports for better code organizationGood practice to use a more descriptive alias
cctypesinstead of the shorterccfor better readability.Also applies to: 16-17
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
Closes #3655
Summary by CodeRabbit
New Features
Refactor
These updates lead to more accurate gas consumption calculations and refined transaction processing for a smoother user experience.