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

vms/platformvm: Remove PeekTxs from Mempool interface #2378

Merged
merged 13 commits into from
Dec 11, 2023

Conversation

dhrubabasu
Copy link
Contributor

@dhrubabasu dhrubabasu commented Nov 27, 2023

Why this should be merged

Simplifies mempool interface in anticipation of verifying each tx on top of each other before putting into a block.

How this works

  • Remove HasTxs + PeekTxs from Mempool interface
  • Add Peek method to Mempool interface
  • Duly update consumers of these methods to use Peek or remove usage altogether

How this was tested

CI

@dhrubabasu dhrubabasu added the cleanup Code quality improvement label Nov 27, 2023
@dhrubabasu dhrubabasu added this to the v1.10.17 milestone Nov 27, 2023
@dhrubabasu dhrubabasu self-assigned this Nov 27, 2023
@dhrubabasu dhrubabasu marked this pull request as draft November 27, 2023 21:26
@dhrubabasu dhrubabasu changed the base branch from dev to move-engine-to-mempool November 27, 2023 21:37
@dhrubabasu dhrubabasu changed the title vms/platformvm: Remove HasTxs and PeekTxs from Mempool interface vms/platformvm: Simplify Mempool interface Nov 27, 2023
Base automatically changed from move-engine-to-mempool to dev November 28, 2023 16:18
@dhrubabasu dhrubabasu changed the base branch from dev to stop-building-block-twice November 28, 2023 16:21
@dhrubabasu dhrubabasu changed the title vms/platformvm: Simplify Mempool interface vms/platformvm: Remove PeekTxs from Mempool interface Nov 29, 2023
@dhrubabasu dhrubabasu marked this pull request as ready for review November 29, 2023 00:33
@StephenButtolph StephenButtolph modified the milestones: v1.10.17, v1.10.18 Dec 1, 2023
@dhrubabasu dhrubabasu marked this pull request as draft December 2, 2023 22:29
@dhrubabasu dhrubabasu marked this pull request as ready for review December 3, 2023 14:16
@abi87 abi87 linked an issue Dec 7, 2023 that may be closed by this pull request
Base automatically changed from stop-building-block-twice to dev December 8, 2023 18:59
@dhrubabasu dhrubabasu changed the base branch from dev to slim-mempool-inft December 11, 2023 00:45
@dhrubabasu dhrubabasu marked this pull request as ready for review December 11, 2023 02:15
Base automatically changed from slim-mempool-inft to dev December 11, 2023 07:21
Copy link
Contributor

@abi87 abi87 left a comment

Choose a reason for hiding this comment

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

just a question about using Pop instead of Peek. Other than that LGTM

@StephenButtolph StephenButtolph added this pull request to the merge queue Dec 11, 2023
@StephenButtolph
Copy link
Contributor

just a question about using Pop instead of Peek. Other than that LGTM

Talked about offline - using Pop followed by re-Adding would change the order of the txs in the mempool. So we're happy with Peek -> Remove.

Merged via the queue into dev with commit 4be5218 Dec 11, 2023
16 checks passed
@StephenButtolph StephenButtolph deleted the remove-peek-txs branch December 11, 2023 17:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cleanup Code quality improvement
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants