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.

Sign up for GitHub

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

Jump to bottom

fix: Removing IgnoreList from Lane Interface #245

Merged
merged 30 commits into from
Nov 29, 2023
Merged

fix: Removing IgnoreList from Lane Interface #245

merged 30 commits into from
Nov 29, 2023

Conversation

davidterpay
Copy link
Contributor

@davidterpay davidterpay commented Nov 28, 2023 •
edited
Loading

This PR deprecates the use of IgnoreLists on lanes - which have been a pita. We can achieve the same outcome with much less code and better separation of concerns by moving what lanes a given lane wants to ignore by moving that logic to the match handler of each lane.

davidterpay and others added 25 commits November 21, 2023 17:46
@davidterpay
greedy approach to lane verification
4abf08e
@davidterpay
docs
b9fc721
@davidterpay
base lane testing
580f487
@davidterpay
mev lane testing nits
0a44f1f
@davidterpay
abci top level testing done
0399069
@davidterpay
network spamming in E2E
8b25b03
@davidterpay
string rep of escrow address
b1a14cd
@davidterpay
nit
6266d60
@davidterpay
nit
ad9df3d
@davidterpay
nit v1.0.1
2084ddf
@davidterpay
removing logs from testing
b337085
@davidterpay
Merge branch 'terpay/greedy-process-proposal' into terpay/escrow-addr…
f6c723e 
…ess-string
@davidterpay
query test
0f8c6f8
@davidterpay
logging with tx info
a562fcd
Merge branch 'terpay/escrow-address-string' into terpay/lane-tx-info
d63bc47
@davidterpay
nits
c95343b
@davidterpay
Merge branch 'main' into terpay/escrow-address-string
a45de07
@davidterpay
Merge branch 'terpay/escrow-address-string' into terpay/lane-tx-info
91b2fa9
@davidterpay
Merge branch 'main' into terpay/lane-tx-info
3026f7f
@davidterpay
nit
765de40
@davidterpay
nit
36ec06a
@davidterpay
testing nit
f20b15b
@davidterpay
adding readmes which i will fill out l8r
d59616c
@davidterpay
removing ignore list from convo, ur done
b05e316
@davidterpay
removing logs in testing
326dd5c
Base automatically changed from terpay/lane-tx-info to main November 28, 2023 18:51
@davidterpay davidterpay marked this pull request as ready for review November 28, 2023 21:09
aljo242
aljo242 reviewed Nov 28, 2023
View reviewed changes
block/mempool_test.go Show resolved Hide resolved
tests/app/lanes.go Outdated Show resolved Hide resolved
nivasan1
nivasan1 reviewed Nov 28, 2023
View reviewed changes
Copy link
Contributor

@nivasan1 nivasan1 left a comment

Choose a reason for hiding this comment

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

I feel like this implementation forces devs to be aware of the implementation details of the other configured lanes. I.e if I have Lane1 and Lane2, but there is a specific tx-type that matches to both, I can no longer have both lanes in my app.

tests/app/lanes.go Show resolved Hide resolved
block/base/handlers.go Show resolved Hide resolved
@davidterpay davidterpay added backport/v1.x.x Backport your PR to the v1.x.x release backport/v2.x.x Backport your PR to the v2.x.x release labels Nov 29, 2023
@davidterpay davidterpay merged commit b91cfb6 into main Nov 29, 2023
8 checks passed
@davidterpay davidterpay deleted the terpay/ignorelist-dep branch November 29, 2023 17:10
mergify bot pushed a commit that referenced this pull request Nov 29, 2023
@davidterpay @mergify
fix: Removing IgnoreList from Lane Interface (#245)
50c5615 
* greedy approach to lane verification

* docs

* base lane testing

* mev lane testing nits

* abci top level testing done

* network spamming in E2E

* string rep of escrow address

* nit

* nit

* nit v1.0.1

* removing logs from testing

* query test

* logging with tx info

* nits

* nit

* nit

* testing nit

* adding readmes which i will fill out l8r

* removing ignore list from convo, ur done

* removing logs in testing

* nits

* eh ig we dont need it rn

* removing ignore list from config

* nit test

---------

Co-authored-by: Alex Johnson <[email protected]>
(cherry picked from commit b91cfb6)

# Conflicts:
#	abci/abci_test.go
#	block/base/lane.go
#	block/lane.go
#	block/mempool.go
#	block/mempool_test.go
#	block/mocks/lane.go
#	block/proposals/proposals_test.go
#	tests/app/app.go
@mergify mergify bot mentioned this pull request Nov 29, 2023
Merged
mergify bot pushed a commit that referenced this pull request Nov 29, 2023
@davidterpay @mergify
fix: Removing IgnoreList from Lane Interface (#245)
8fad2db 
* greedy approach to lane verification

* docs

* base lane testing

* mev lane testing nits

* abci top level testing done

* network spamming in E2E

* string rep of escrow address

* nit

* nit

* nit v1.0.1

* removing logs from testing

* query test

* logging with tx info

* nits

* nit

* nit

* testing nit

* adding readmes which i will fill out l8r

* removing ignore list from convo, ur done

* removing logs in testing

* nits

* eh ig we dont need it rn

* removing ignore list from config

* nit test

---------

Co-authored-by: Alex Johnson <[email protected]>
(cherry picked from commit b91cfb6)

# Conflicts:
#	abci/abci_test.go
#	block/base/lane.go
#	block/lane.go
#	block/mempool.go
#	block/mempool_test.go
#	block/mocks/lane.go
#	block/proposals/proposals_test.go
#	tests/app/app.go
@mergify mergify bot mentioned this pull request Nov 29, 2023
Merged
davidterpay added a commit that referenced this pull request Dec 1, 2023
@mergify @davidterpay
fix: Removing IgnoreList from Lane Interface (backport #245) (#249)
fd8a786 
* fix: Removing IgnoreList from Lane Interface (#245)

* greedy approach to lane verification

* docs

* base lane testing

* mev lane testing nits

* abci top level testing done

* network spamming in E2E

* string rep of escrow address

* nit

* nit

* nit v1.0.1

* removing logs from testing

* query test

* logging with tx info

* nits

* nit

* nit

* testing nit

* adding readmes which i will fill out l8r

* removing ignore list from convo, ur done

* removing logs in testing

* nits

* eh ig we dont need it rn

* removing ignore list from config

* nit test

---------

Co-authored-by: Alex Johnson <[email protected]>
(cherry picked from commit b91cfb6)

# Conflicts:
#	abci/abci_test.go
#	block/base/lane.go
#	block/lane.go
#	block/mempool.go
#	block/mempool_test.go
#	block/mocks/lane.go
#	block/proposals/proposals_test.go
#	tests/app/app.go

* why are there so many conflicts

* more conflicts

* fixing merge

---------

Co-authored-by: David Terpay <[email protected]>
Co-authored-by: David Terpay <[email protected]>
davidterpay added a commit that referenced this pull request Dec 1, 2023
@mergify @davidterpay
fix: Removing IgnoreList from Lane Interface (backport #245) (#250)
b9facbc 
* fix: Removing IgnoreList from Lane Interface (#245)

* greedy approach to lane verification

* docs

* base lane testing

* mev lane testing nits

* abci top level testing done

* network spamming in E2E

* string rep of escrow address

* nit

* nit

* nit v1.0.1

* removing logs from testing

* query test

* logging with tx info

* nits

* nit

* nit

* testing nit

* adding readmes which i will fill out l8r

* removing ignore list from convo, ur done

* removing logs in testing

* nits

* eh ig we dont need it rn

* removing ignore list from config

* nit test

---------

Co-authored-by: Alex Johnson <[email protected]>
(cherry picked from commit b91cfb6)

# Conflicts:
#	abci/abci_test.go
#	block/base/lane.go
#	block/lane.go
#	block/mempool.go
#	block/mempool_test.go
#	block/mocks/lane.go
#	block/proposals/proposals_test.go
#	tests/app/app.go

* why are there so many conflicts

* fixing merge

* timing in e2e

---------

Co-authored-by: David Terpay <[email protected]>
Co-authored-by: David Terpay <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nivasan1 nivasan1 nivasan1 left review comments

@aljo242 aljo242 aljo242 approved these changes

@Eric-Warehime Eric-Warehime Awaiting requested review from Eric-Warehime Eric-Warehime is a code owner

Assignees
No one assigned
Labels
backport/v1.x.x Backport your PR to the v1.x.x release backport/v2.x.x Backport your PR to the v2.x.x release
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@davidterpay @aljo242 @nivasan1