Skip to content

fix: retryable excess gas refund#635

Closed
gzeoneth wants to merge 4 commits intomasterfrom
fix-retryable-refund
Closed

fix: retryable excess gas refund#635
gzeoneth wants to merge 4 commits intomasterfrom
fix-retryable-refund

Conversation

@gzeoneth
Copy link
Member

@gzeoneth gzeoneth commented May 24, 2022

When we have _gasPriceBid higher than actual L2 gas price, or otherwise have deposit > l2callvalue + maxSubmissionCost + gascost, our refund is insufficient because we only refund gasPrice * gasLeft, even if gasPrice < _gasPriceBid

refund := arbmath.BigMulByUint(gasPrice, gasLeft)

We should refund excess gas provided from L1, i.e.
max(deposit - l2callvalue - maxSubmissionCost - gascost, 0)

The retryable can use existing fund in L2 from account to provide gas, but the refund will be bounded by excessDeposit - gascost.

@codecov
Copy link

codecov bot commented May 24, 2022

Codecov Report

Merging #635 (349b1ba) into master (69fd4e3) will decrease coverage by 4.64%.
The diff coverage is 75.00%.

@@            Coverage Diff             @@
##           master     #635      +/-   ##
==========================================
- Coverage   52.32%   47.67%   -4.65%     
==========================================
  Files         210      199      -11     
  Lines       25080    21065    -4015     
  Branches      486      486              
==========================================
- Hits        13122    10042    -3080     
+ Misses      10635     9699     -936     
- Partials     1323     1324       +1     

@gzeoneth gzeoneth marked this pull request as ready for review May 24, 2022 09:57
@PlasmaPower
Copy link
Contributor

I'm not sure we want to move excess deposit to the fee refund address. I thought the idea was that the deposit would go to the L2 address.

@gzeoneth
Copy link
Member Author

gzeoneth commented May 26, 2022

I'm not sure we want to move excess deposit to the fee refund address. I thought the idea was that the deposit would go to the L2 address.

well that's the behaviour in classic and also documented
https://developer.offchainlabs.com/docs/l1_l2_messages#other-parameters

Credit-Back Address: Address to which all excess gas is credited on L2; i.e., excess ETH for base submission cost (MaxSubmissionCost - ActualSubmissionCostPaid) and excess ETH provided for L2 execution ((GasPrice x MaxGas) - ActualETHSpentInExecution).

or do you mean we should refund max(min(deposit - l2callvalue - maxSubmissionCost, gasprice * maxgas) - gascost, 0) instead

@gzeoneth
Copy link
Member Author

gzeoneth commented May 31, 2022

discussed offline, I think the conclusion is to refund excessSubmissionCost + gasprice * maxgas - gascost
constrained by gasrefund + l2callvalue + maxSubmissionCost <= deposit

@gzeoneth gzeoneth marked this pull request as draft May 31, 2022 19:56
@gzeoneth gzeoneth closed this Jun 2, 2022
@gzeoneth gzeoneth deleted the fix-retryable-refund branch June 2, 2022 18:27
tsahee pushed a commit that referenced this pull request Jul 29, 2025
Co-authored-by: gzeon <hng@offchainlabs.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants