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.

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

Use ring buffer for pending Ether Deposits #13783

Closed
dantaik opened this issue May 19, 2023 · 8 comments · Fixed by #13868
Closed

Use ring buffer for pending Ether Deposits #13783

dantaik opened this issue May 19, 2023 · 8 comments · Fixed by #13868

Comments

@dantaik
Copy link
Contributor

dantaik commented May 19, 2023

As of now, the EthDeposit[] ethDeposits is an ever growing array. It can be turned into a ring buffer for reducing gas cost. This will be a very good first issue for community developer to implement.

@Venoox
Copy link
Contributor

Venoox commented May 21, 2023

I'd like to try this
Do you think it's a good idea to implement this with a fixed size array and having two variables to store the index of the start and the end of the buffer?

@cyberhorsey
Copy link
Contributor

I'd like to try this Do you think it's a good idea to implement this with a fixed size array and having two variables to store the index of the start and the end of the buffer?

I think we can just modulo ID by the ringBufferSize? Haven't looked at the code much for this but I think that's how our other ring buffers are implemented.

@Venoox
Copy link
Contributor

Venoox commented May 22, 2023

When the buffer is full and a new deposit is made, we overwrite the oldest deposit, is that correct?

I think we can just modulo ID by the ringBufferSize? Haven't looked at the code much for this but I think that's how our other ring buffers are implemented.

In this case I think it's different because we need to track the oldest deposit because when we process deposits we go from the oldest to the newest. So we need at least size and nextIndex variable to keep track of everything.

@dantaik dantaik moved this from 📝 Todo to 🏗 In progress in Taiko Project Board May 27, 2023
@dantaik
Copy link
Contributor Author

dantaik commented May 27, 2023

@Venoox I assigned this feature to you. Do you think you can give us an ETA?

@Venoox
Copy link
Contributor

Venoox commented May 29, 2023

A few days would be my best guess

@dantaik
Copy link
Contributor Author

dantaik commented Jun 3, 2023

A few days would be my best guess

I got it done here: #13868

@Venoox
Copy link
Contributor

Venoox commented Jun 4, 2023

Oh I've just finished working on it :/

@RogerRabbitTH
Copy link

Good

@github-project-automation github-project-automation bot moved this from 🏗 In progress to ✅ Done in Taiko Project Board Jun 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

5 participants