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

Refactor TxGraph::walk_conflicts to check for ancestors #1100

Closed

Conversation

LagginTimes
Copy link
Contributor

@LagginTimes LagginTimes commented Aug 28, 2023

Description

Implementation for #1099.

This PR builds off of #1064 and refactors TxGraph::walk_conflicts to check for conflicts in ancestor transactions by using a new TxAncestors iterator in TxGraph.

Changelog notice

  • Introduced TxGraph::TxAncestors iterator
  • Refactored TxGraph::walk_conflicts to use TxGraph::TxAncestors
  • Added walk_ancestors to TxGraph

All Submissions:

  • I've signed all my commits
  • I followed the contribution guidelines
  • I ran cargo fmt and cargo clippy before committing

New Features:

  • I've added tests for the new feature
  • I've added docs for the new feature

@LagginTimes LagginTimes marked this pull request as draft August 28, 2023 12:09
@LagginTimes LagginTimes force-pushed the refactor_walk_conflicts branch 2 times, most recently from 105df7a to 85fdfae Compare August 28, 2023 14:14
@LagginTimes LagginTimes marked this pull request as ready for review August 28, 2023 14:21
@LagginTimes LagginTimes force-pushed the refactor_walk_conflicts branch 2 times, most recently from f61b79e to eaefc3e Compare August 29, 2023 07:39
The failing tests are commented out.

Signed-off-by: Wei Chen <wzc110@gmail.com>
`TxGraph::try_get_chain_position` traverses ancestors so we do
not skip conflicts.

fix(chain): add check for ancestor tx conflicts in `TxGraph`

Signed-off-by: Wei Chen <wzc110@gmail.com>
Signed-off-by: Wei Chen <wzc110@gmail.com>
@LagginTimes LagginTimes force-pushed the refactor_walk_conflicts branch from 73dc671 to 24e9cdf Compare August 31, 2023 08:39
@evanlinjin
Copy link
Member

Closing as these changes are merged into #1064

@evanlinjin evanlinjin closed this Sep 1, 2023
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.

2 participants