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

Revert PR 249 #252

Merged
merged 5 commits into from
Sep 19, 2022
Merged

Revert PR 249 #252

merged 5 commits into from
Sep 19, 2022

Conversation

holisticode
Copy link
Contributor

@holisticode holisticode commented Sep 5, 2022

No description provided.

Copy link
Contributor

@felipemadero felipemadero left a comment

Choose a reason for hiding this comment

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

LGTM

@ramilexe
Copy link

ramilexe commented Sep 6, 2022

Can we get it merged asap?

@hexfusion
Copy link
Contributor

Just FYI we could consider nixing the shared-timer[1] logic which is now back as part of avalanchego. We should come up with a general plan for replacement as the it is defined as deprecated but just a thought before this merges.

[1] https://github.com/ava-labs/subnet-evm/pull/249/files#diff-08c0341b9a66a2439b8a1ae1403c49af7621c34d89d3e0b0023c11077d0463ed

@aaronbuchwald
Copy link
Collaborator

Just FYI we could consider nixing the shared-timer[1] logic which is now back as part of avalanchego. We should come up with a general plan for replacement as the it is defined as deprecated but just a thought before this merges.

[1] https://github.com/ava-labs/subnet-evm/pull/249/files#diff-08c0341b9a66a2439b8a1ae1403c49af7621c34d89d3e0b0023c11077d0463ed

Good callout, adding an issue to track this here. We should be able to remove all timeouts from the block building logic. We have already gotten rid of the two staged timer in coreth, which will be migrated to subnet-evm soon.

@aaronbuchwald aaronbuchwald merged commit e93fc79 into master Sep 19, 2022
@aaronbuchwald aaronbuchwald deleted the revert-PR-249 branch September 19, 2022 16:25
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.

7 participants