Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Debug Gas Escalator (Legacy Transactions) #4108

Closed
daniel-savu opened this issue Jul 9, 2024 · 0 comments · Fixed by #4211
Closed

Debug Gas Escalator (Legacy Transactions) #4108

daniel-savu opened this issue Jul 9, 2024 · 0 comments · Fixed by #4211
Assignees
@daniel-savu
Labels
relayer

Comments

@daniel-savu
Copy link
Contributor

Problem

We recently tried rolling out the ethers gas escalator middleware for legacy txs and ran into the following:

  • it's really weird that the escalator waits for less than a second in some cases before bumping the gas price, and I found a tx that had its gas bumped 40 times, starting from gas price 2000000000 to 222398008517 (a 100x increase)
    https://cloudlogging.app.goo.gl/jfa2oqtGGS1yJW2v9
  • also turns out the escalator is also a memory hog (28gb), reverted the PR and scaled down RC. We already have some ideas but will wait until after eth cc
@daniel-savu daniel-savu self-assigned this Jul 9, 2024
@daniel-savu daniel-savu moved this to Sprint in Hyperlane Tasks Jul 9, 2024
@daniel-savu daniel-savu moved this from Sprint to In Progress in Hyperlane Tasks Jul 29, 2024
@daniel-savu daniel-savu mentioned this issue Dec 3, 2024
Merged
github-merge-queue bot pushed a commit that referenced this issue Dec 3, 2024
@daniel-savu @tkporter
feat: Gas escalator middleware (#4211)
d0e53f5 
### Description

Follow up to
#3852 and
#4098.

This PR brings major fixes to the escalator:
- A revamped escalator implementation, that was forked from ethers-rs
and had various fixes applied to it. The most notable bug caused
"infinite" gas price escalations to occur within a very short timespan,
until the max configured gas price was reached. Full diff here:
hyperlane-xyz/ethers-rs@950dd34...edf703a
- Support for escalating EIP-1559 txs, in addition to the existing
Legacy tx type support
- Memory leak fix. Since each escalator instance spawns a thread and we
create transient providers for each message, this created a memory leak
of escalator tasks that would build up over time - on RC, memory usage
had reached 27gb before we noticed. This PR merges in the changes from
#4208 to stop
the leak. Profiling results:
- before the fix (62mb peak usage): ![Screenshot_2024-07-29_at_12 19
26](https://github.com/user-attachments/assets/5b52fecb-6733-481c-96fb-10f505b9bf8a)
- after the fix (16mb peak usage): ![Screenshot_2024-07-29_at_12 19
53](https://github.com/user-attachments/assets/a78b9452-78cb-4b5b-9aa6-fe7f3a6ac683)

### Drive-by changes

<!--
Are there any minor or drive-by changes also included?
-->

### Related issues

- Fixes #4108

### Backward compatibility

Yes

### Testing

Manual testing using the unit test setup from ethers-rs, and using it on
RC to observe it in the wild. Prom chain returns a `could not replace
existing tx` error when a tx is resubmitted so the escalator won't be
effective there, but this can be fixed later

---------

Co-authored-by: Trevor Porter <tkporter4@gmail.com>
@github-project-automation github-project-automation bot moved this from In Progress to Done in Hyperlane Tasks Dec 3, 2024
tiendn pushed a commit to tiendn/hyperlane-monorepo that referenced this issue Jan 11, 2025
@daniel-savu @tkporter @tiendn
feat: Gas escalator middleware (hyperlane-xyz#4211)
78ab97a 
### Description

Follow up to
hyperlane-xyz#3852 and
hyperlane-xyz#4098.

This PR brings major fixes to the escalator:
- A revamped escalator implementation, that was forked from ethers-rs
and had various fixes applied to it. The most notable bug caused
"infinite" gas price escalations to occur within a very short timespan,
until the max configured gas price was reached. Full diff here:
hyperlane-xyz/ethers-rs@950dd34...edf703a
- Support for escalating EIP-1559 txs, in addition to the existing
Legacy tx type support
- Memory leak fix. Since each escalator instance spawns a thread and we
create transient providers for each message, this created a memory leak
of escalator tasks that would build up over time - on RC, memory usage
had reached 27gb before we noticed. This PR merges in the changes from
hyperlane-xyz#4208 to stop
the leak. Profiling results:
- before the fix (62mb peak usage): ![Screenshot_2024-07-29_at_12 19
26](https://github.com/user-attachments/assets/5b52fecb-6733-481c-96fb-10f505b9bf8a)
- after the fix (16mb peak usage): ![Screenshot_2024-07-29_at_12 19
53](https://github.com/user-attachments/assets/a78b9452-78cb-4b5b-9aa6-fe7f3a6ac683)

### Drive-by changes

<!--
Are there any minor or drive-by changes also included?
-->

### Related issues

- Fixes hyperlane-xyz#4108

### Backward compatibility

Yes

### Testing

Manual testing using the unit test setup from ethers-rs, and using it on
RC to observe it in the wild. Prom chain returns a `could not replace
existing tx` error when a tx is resubmitted so the escalator won't be
effective there, but this can be fixed later

---------

Co-authored-by: Trevor Porter <tkporter4@gmail.com>
tiendn pushed a commit to tiendn/hyperlane-monorepo that referenced this issue Jan 11, 2025
@daniel-savu @tkporter @tiendn
feat: Gas escalator middleware (hyperlane-xyz#4211)
f510d93 
### Description

Follow up to
hyperlane-xyz#3852 and
hyperlane-xyz#4098.

This PR brings major fixes to the escalator:
- A revamped escalator implementation, that was forked from ethers-rs
and had various fixes applied to it. The most notable bug caused
"infinite" gas price escalations to occur within a very short timespan,
until the max configured gas price was reached. Full diff here:
hyperlane-xyz/ethers-rs@950dd34...edf703a
- Support for escalating EIP-1559 txs, in addition to the existing
Legacy tx type support
- Memory leak fix. Since each escalator instance spawns a thread and we
create transient providers for each message, this created a memory leak
of escalator tasks that would build up over time - on RC, memory usage
had reached 27gb before we noticed. This PR merges in the changes from
hyperlane-xyz#4208 to stop
the leak. Profiling results:
- before the fix (62mb peak usage): ![Screenshot_2024-07-29_at_12 19
26](https://github.com/user-attachments/assets/5b52fecb-6733-481c-96fb-10f505b9bf8a)
- after the fix (16mb peak usage): ![Screenshot_2024-07-29_at_12 19
53](https://github.com/user-attachments/assets/a78b9452-78cb-4b5b-9aa6-fe7f3a6ac683)

### Drive-by changes

<!--
Are there any minor or drive-by changes also included?
-->

### Related issues

- Fixes hyperlane-xyz#4108

### Backward compatibility

Yes

### Testing

Manual testing using the unit test setup from ethers-rs, and using it on
RC to observe it in the wild. Prom chain returns a `could not replace
existing tx` error when a tx is resubmitted so the escalator won't be
effective there, but this can be fixed later

---------

Co-authored-by: Trevor Porter <tkporter4@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@daniel-savu daniel-savu

Labels
relayer
Projects
Hyperlane Tasks
Archived in project
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

feat: Gas escalator middleware hyperlane-xyz/hyperlane-monorepo
1 participant
@daniel-savu