search

sherlock-audit / 2024-08-tokamak-network-judging

1 stars 0 forks source link

Add RELAY_GAS_CHECK_BUFFER_INCLUDING_APPROVAL instead of increasing RELAY_GAS_CHECK_BUFFER #89

Open theo-learner opened 1 month ago

theo-learner commented 1 month ago

This issue is reported by group of Issue #41. It is unvalid (low severity). But protocol team decided to change the code for it as below.

It seems better if adding other variable (RELAY_GAS_CHECK_BUFFER_INCLUDING_APPROVAL)on L2 instead of increasing RELAY_GAS_CHECK_BUFFER for both L1, L2.

uint64 public constant RELAY_GAS_CHECK_BUFFER_INCLUDING_APPROVAL = 40_000;

We don't need to increase gas for L1CrossDomainMessenger.sendMessage or L1CrossDomainMessenger.sendNativeTokenMessage ( will be relayed by L2CrossDomainMessenger.relayMessage), it would be a waste because of burning step. https://github.com/tokamak-network/tokamak-thanos/blob/main/packages/tokamak/contracts-bedrock/src/L1/OptimismPortal2.sol#L524

theo-learner commented 1 month ago

Note: this issue is self-reported by protocol team (Tokamak Network)

sherlock-admin2 commented 1 month ago

The protocol team fixed this issue in the following PRs/commits: https://github.com/tokamak-network/tokamak-thanos/pull/287