feat: add gas limit for sumitBridgeCall #843
Conversation
WalkthroughThis pull request introduces a comprehensive enhancement to the cross-chain bridge functionality by adding a Changes
Sequence DiagramsequenceDiagram
participant Client
participant BridgeContract
participant Validator
Client->>BridgeContract: bridgeCall(data, gasLimit)
BridgeContract->>Validator: Validate gasLimit
Validator-->>BridgeContract: Validation Result
BridgeContract->>BridgeContract: Process Bridge Call
BridgeContract-->>Client: Bridge Call Executed
Possibly related PRs
Poem
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
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
x/crosschain/keeper/bridge_call_out.go (1)
Line range hint
29-38: Validate gasLimit and eventNonce usage in AddOutgoingBridgeCall.
This method's signature has grown, now includinggasLimitandeventNonce. It's advisable to validategasLimitdoesn't exceed any configured or recommended upper bound and confirm the correctness ofeventNonce(e.g., verifying it's non-zero if required). This ensures tighter control over resource usage and consistency across the codebase.contract/ifx_bridge_logic.sol.go (2)
63-63: ABI changes to include _gasLimit.
The updated ABI correctly reflects the new gas limit parameter in bridging methods. Ensure all dependent tools and deployments are updated (e.g., front-ends, off-chain signers, or UI components) to handle the new parameter.Would you like help generating a migration checklist for dependencies that parse or interact with this ABI?
916-932: Extended SubmitBridgeCall with new BridgeCallData fields.
The structure_inputnow containsGasLimit. Confirm that all off-chain or on-chain logic employingSubmitBridgeCallis updated to supply or manage gas limit constraints. Failure to do so might result in unexpected reverts or partial success.solidity/contracts/bridge/IFxBridgeLogic.sol (1)
59-59: Introducing gasLimit field in BridgeCallData struct.
This field aligns with the crosschain adjustments, but confirm any front-ends or services constructingBridgeCallDataalso provide a valid gas limit. If the contract logic allows a default or fallback, consider clarifying it in documentation.docs/swagger-ui/swagger.yaml (1)
5834-5836: Add documentation for the gas_limit field.Consider adding a description field to document:
- The purpose of gas_limit
- Any constraints or valid ranges
- Example values
Add documentation like this:
gas_limit: type: string format: uint64 + description: Specifies the maximum amount of gas that can be used for the bridge call execution + example: "3000000" + minimum: "0"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
x/crosschain/types/types.pb.gois excluded by!**/*.pb.go
📒 Files selected for processing (10)
api/fx/gravity/crosschain/v1/types.pulsar.go(16 hunks)contract/contract.go(2 hunks)contract/crosschain.go(1 hunks)contract/ifx_bridge_logic.sol.go(9 hunks)docs/swagger-ui/swagger.yaml(11 hunks)proto/fx/gravity/crosschain/v1/types.proto(1 hunks)solidity/contracts/bridge/IFxBridgeLogic.sol(2 hunks)x/crosschain/keeper/bridge_call_out.go(5 hunks)x/crosschain/types/types.go(1 hunks)x/crosschain/types/types_test.go(1 hunks)
🔇 Additional comments (28)
x/crosschain/keeper/bridge_call_out.go (4)
41-41: Double-check success path handling after BuildOutgoingBridgeCall.
After building an outgoing bridge call, consider checking any additional invariants (e.g., verifying gasLimit again, if there's a maximum or re-check logic). Also, consider adding logs or metrics capturing the new gasLimit parameter for easier tracking.
Line range hint 79-95: Consistent usage of gasLimit in BuildOutgoingBridgeCall.
Here, the function includes a new gasLimit parameter. Be sure this is always aligned with other checks in the system (like max gas limit from parameters). The rest of the logic for constructing the OutgoingBridgeCall object looks consistent. Ensure any future changes in gasLimit validation are uniformly applied throughout this code path.
97-97: GasLimit assignment.
The assignment of GasLimit: gasLimit is straightforward. Just confirm that higher-level validation (e.g., in a Validate() method) or parameter constraints exist to catch any malformed or excessively large values.
292-292: Event nonce hard-coded to 0 in BridgeCallBaseCoin.
While passing gasLimit.Uint64() is correct, always hard-coding eventNonce to 0 might cause confusion or break event-order assumptions elsewhere. Verify whether this is intentional or if a real event nonce should be determined before invocation.
contract/ifx_bridge_logic.sol.go (3)
42-42: New GasLimit field in IFxBridgeLogicBridgeCallData.
This addition aligns with the bridging changes, enabling explicit gas allocation. Confirm that inbound calls properly set or clamp this value to reduce potential out-of-gas or exceeding-limit issues.
769-787: BridgeCallCheckpoint extended with _gasLimit.
Including _gasLimit in bridgeCallCheckpoint helps unify cross-chain message consistency. Verify that the new field is accounted for in the checkpoint logic (e.g., hashing/encoding). A mismatch in the checkpoint payload vs. contract expectation could break bridging.
Line range hint 1233-1313: Enhanced BridgeCallEvent logic for GasLimit.
Filtering and parsing now incorporate the _gasLimit index in the event. Ensure consistent indexing in event watchers or logs-based off-chain tooling. Missing or mismatched indexing typically leads to decoding failures or inaccurate metrics.
contract/crosschain.go (1)
73-75: GasLimit validation checks.
This validation guards against negative or overly large values (via IsUint64()). Consider also checking for zero if that's invalid in your context. This ensures bridging calls do not proceed with nonsensical or harmful gas limit values.
solidity/contracts/bridge/IFxBridgeLogic.sol (1)
194-194: bridgeCallCheckpoint gains _gasLimit.
Revisiting the architecture that calculates or references _gasLimit is essential to maintain bridging correctness. Ensure all references to this updated signature consistently pass the correct argument.
x/crosschain/types/types_test.go (2)
81-81: Consider verifying a more realistic GasLimit.
A gas limit of 1 might be too small to simulate real-world usage and could potentially mask out-of-gas scenarios in production-like scenarios.
87-87: Updated expected checkpoint looks good.
The new hash appears consistent with the inclusion of the GasLimit field in the checkpoint.
x/crosschain/types/types.go (1)
474-474: Be cautious with uint64 to int64 conversion.
Here, m.GasLimit is cast to int64. If GasLimit can exceed 2^63 - 1, this may cause an overflow. Consider validating the maximum allowable GasLimit before conversion.
func (m *OutgoingBridgeCall) GetCheckpoint(gravityIDString string) ([]byte, error) {
...
- big.NewInt(int64(m.GasLimit)),
+ if m.GasLimit > math.MaxInt64 {
+ return nil, fmt.Errorf("gas limit overflow: %d > MaxInt64", m.GasLimit)
+ }
+ gasLimitBigInt := big.NewInt(int64(m.GasLimit))
...
}contract/contract.go (2)
187-187: Signature update to include gasLimit is appropriate.
This reflects the new requirement to handle gas limits in the checkpoint. Ensure the caller provides a value that aligns with actual chain constraints.
200-200: Correctly forwarding the gasLimit parameter.
Passing gasLimit into the bridgeCallCheckpoint function ensures the encoded call accurately reflects the new feature.
api/fx/gravity/crosschain/v1/types.pulsar.go (13)
6745-6745: New field declarations added for OutgoingBridgeCall message
The code adds new field descriptors for gas_limit and event_nonce fields in the OutgoingBridgeCall message type.
Also applies to: 6761-6761
6884-6889: Gas limit field handling added to Range method
The Range method implementation is updated to handle the new gas_limit field for the OutgoingBridgeCall message type. The implementation correctly checks and returns the field value.
6929-6930: Gas limit field check added to Has method
The Has method is updated to check for the presence of the gas_limit field in the OutgoingBridgeCall message type.
6967-6968: Gas limit field clearing added to Clear method
The Clear method is updated to handle clearing of the gas_limit field in the OutgoingBridgeCall message type.
7017-7019: Gas limit field getter added to Get method
The Get method is updated to handle retrieving the value of the gas_limit field from the OutgoingBridgeCall message type.
7063-7064: Gas limit field setter added to Set method
The Set method is updated to handle setting the value of the gas_limit field in the OutgoingBridgeCall message type.
7109-7110: Gas limit field mutability check added to Mutable method
The Mutable method is updated to handle mutability checks for the gas_limit field in the OutgoingBridgeCall message type.
7145-7146: Gas limit field default value handling added to NewField method
The NewField method is updated to handle providing the default value for the gas_limit field in the OutgoingBridgeCall message type.
7253-7255: Gas limit field size calculation added to Size method
The Size method is updated to include the gas_limit field in the size calculation for the OutgoingBridgeCall message type.
7291-7295: Gas limit field marshaling added to Marshal method
The Marshal method is updated to handle serializing the gas_limit field for the OutgoingBridgeCall message type.
7665-7682: Gas limit field unmarshaling added to Unmarshal method
The Unmarshal method is updated to handle deserializing the gas_limit field for the OutgoingBridgeCall message type.
Line range hint 9548-9570: Updated message definition for OutgoingBridgeCall
The protobuf message definition for OutgoingBridgeCall is updated to include the new gas_limit field. The field is properly defined with the correct type and field number.
9565-9569: Gas limit field encoding added to protobuf definition
The protobuf encoding details for the gas_limit field are properly defined, including the field number and wire format encoding.
docs/swagger-ui/swagger.yaml (1)
5834-5836: LGTM! Consistent implementation of gas_limit field.
The gas_limit field has been consistently added across all relevant sections with appropriate type (string) and format (uint64), matching the pattern used for other numeric fields like block_height and event_nonce.
Also applies to: 6091-6093, 6813-6815, 7149-7151, 8365-8367, 42307-42309, 42527-42529, 42570-42572, 42655-42657, 42724-42726, 42977-42979
Summary by CodeRabbit
Release Notes
New Features
gas_limitfield for bridge calls, enhancing gas management.gas_limitparameter.Bug Fixes
GasLimitto ensure it is non-negative and valid.Documentation
gas_limitfield across multiple definitions.Tests
GasLimitfield in theOutgoingBridgeCallstructure.