Skip to content

fix(alpen-cli): reserve gas for alpen drain#1931

Open
voidash wants to merge 1 commit into
mainfrom
fix/alpen-cli-drain-gas
Open

fix(alpen-cli): reserve gas for alpen drain#1931
voidash wants to merge 1 commit into
mainfrom
fix/alpen-cli-drain-gas

Conversation

@voidash
Copy link
Copy Markdown
Contributor

@voidash voidash commented Jun 3, 2026

Summary

  • reserve Alpen drain gas using wide U256 arithmetic
  • submit the drain transaction with the same explicit gas_limit and gas_price used for the balance calculation
  • add focused tests for exact-fee, below-fee, and wide-arithmetic cases

Bug Encountered

During the local k2 alpen-cli smoke run, alpen drain --alpen-address 0x000000000000000000000000000000000000c0Fe failed after the CLI wallet had been funded and had already sent an Alpen transfer. The RPC rejected the drain transaction with:

insufficient funds for gas * price + value: have 998966468221608000 want 998979000000000000

The old code estimated gas for a 1-wei transaction, computed balance - gas_estimate * gas_price, then submitted a different max-value transaction while leaving the provider to fill gas fields again. That allowed the submitted transaction's upfront gas requirement to exceed the amount reserved by the drain calculation.

This PR fixes that by calculating the reserved fee with U256, returning early when there is no spendable balance after gas, and setting gas_limit and gas_price explicitly on the submitted drain transaction.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 3, 2026

Commit: 99c6d00

SP1 Execution Results

program cycles gas
EVM EE Chunk 824,732 969,395
EVM EE Account 404,056 498,593
Checkpoint 2,601,525 3,007,269

@codecov
Copy link
Copy Markdown

codecov Bot commented Jun 3, 2026

Codecov Report

❌ Patch coverage is 93.75000% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 69.78%. Comparing base (55907ce) to head (e3b82c7).
⚠️ Report is 7 commits behind head on main.

Files with missing lines Patch % Lines
bin/alpen-cli/src/cmd/drain.rs 93.75% 4 Missing ⚠️

❗ There is a different number of reports uploaded between BASE (55907ce) and HEAD (e3b82c7). Click for more details.

HEAD has 2 uploads less than BASE
Flag BASE (55907ce) HEAD (e3b82c7)
functional 2 0
@@             Coverage Diff             @@
##             main    #1931       +/-   ##
===========================================
- Coverage   84.31%   69.78%   -14.54%     
===========================================
  Files         633      631        -2     
  Lines       75681    76312      +631     
===========================================
- Hits        63812    53254    -10558     
- Misses      11869    23058    +11189     
Flag Coverage Δ
functional ?
unit 69.78% <93.75%> (+0.08%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
bin/alpen-cli/src/cmd/drain.rs 34.48% <93.75%> (+34.48%) ⬆️

... and 341 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@voidash voidash marked this pull request as ready for review June 3, 2026 07:47
@voidash voidash requested review from Zk2u and krsnapaudel as code owners June 3, 2026 07:47
Copy link
Copy Markdown
Member

@storopoli storopoli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK cc7c519

@barakshani
Copy link
Copy Markdown
Contributor

We seem to have issues with gas calculation in general, not only in drain -- I'm not sure if this general issue is only when we haven't had sufficient activity (tx history is totally empty or in some recent block range). I think we just don't query what is the minimal gas the network accepts.
Shouldn't we try to fix it more holistically?

@voidash
Copy link
Copy Markdown
Contributor Author

voidash commented Jun 8, 2026

@barakshani

We seem to have issues with gas calculation in general, not only in drain -- I'm not sure if this general issue is only when we haven't had sufficient activity (tx history is totally empty or in some recent block range). I think we just don't query what is the minimal gas the network accepts.
Shouldn't we try to fix it more holistically?

This is only for drain specific invariant so i believe this is acceptable for narrow drain correctness fix. Activity history stuff is not the root issue
Before this pr we didn't set gas_limit(gas_estimate) and gas price. We sent with only to and value.
This would have been broken even if the chain had lots of activity or no activity.

But i agree that we need something on AlpenWallet that is a shared transaction gas helper.

@voidash voidash force-pushed the fix/alpen-cli-drain-gas branch from e3b82c7 to cc7c519 Compare June 8, 2026 08:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants