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

Have auto and interval mining at the same time #7901

Closed
Tracked by #8269
DefiCake opened this issue May 9, 2024 · 8 comments
Closed
Tracked by #8269

Have auto and interval mining at the same time #7901

DefiCake opened this issue May 9, 2024 · 8 comments
Assignees
Labels
C-anvil Command: anvil good first issue Good for newcomers T-feature Type: feature

Comments

@DefiCake
Copy link

DefiCake commented May 9, 2024

Component

Anvil

Describe the feature you would like

Use case: Have fast transactions (= fast CI pipelines and tests) with automated software that reacts on events, like bots, indexers, etc

At the moment anvil allows either one mining mode or the other - auto, or interval. This is specifically signaled by the interval mining mode, that warns that auto is disabled.

Additional context

No response

@DefiCake DefiCake added the T-feature Type: feature label May 9, 2024
@DaniPopes DaniPopes added the C-anvil Command: anvil label May 9, 2024
@mattsse mattsse added the good first issue Good for newcomers label Jun 21, 2024
@mattsse
Copy link
Member

mattsse commented Jun 21, 2024

this should be possible to support by adding a new variant that combines both of these:

/// A miner that listens for new transactions that are ready.
///
/// Either one transaction will be mined per block, or any number of transactions will be
/// allowed
Auto(ReadyTransactionMiner),
/// A miner that constructs a new block every `interval` tick
FixedBlockTime(FixedBlockTimeMiner),

@caiquejjx
Copy link
Contributor

@mattsse is this high priority? I would like to work on it but it's my first time interacting with foundry so it will take some time to get this done

@mattsse
Copy link
Member

mattsse commented Jun 25, 2024

not high priority, the task here would be to add a new variant that is

Mixed {
  auto: ReadyTransactionMiner,
  fixed: FixedBlockTimeMiner
}

and then polls both

@caiquejjx
Copy link
Contributor

oh looks simple enough, thanks!

@caiquejjx
Copy link
Contributor

caiquejjx commented Jun 27, 2024

@mattsse hey, I created a draft pr with the new variant + it's pooling but I'm wondering if I should also add RPC handler for setting the mixed mining mode and besides that I'm not sure how to test this feature, when you have some time can you take a look?
Thank you

@zerosnacks
Copy link
Member

Hi @caiquejjx thanks for picking this up, draft is looking good!

I think it makes sense to add RPC methods for this (anvil_setMixedmine(bool), anvil_getMixedmine(bool) matching anvil_setAutomine(bool) / anvil_getAutomine(bool)).

cc @mattsse

@caiquejjx
Copy link
Contributor

thanks @zerosnacks, I'll follow up with the RPC methods implementation then

@zerosnacks
Copy link
Member

zerosnacks commented Jul 24, 2024

Marking as resolved by #8280

Thanks for your PR @caiquejjx!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-anvil Command: anvil good first issue Good for newcomers T-feature Type: feature
Projects
None yet
Development

No branches or pull requests

5 participants