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

new routing exchange prototype using IBF #4112

Merged
merged 6 commits into from
Sep 25, 2021
Merged

new routing exchange prototype using IBF #4112

merged 6 commits into from
Sep 25, 2021

Conversation

pmnoxx
Copy link
Contributor

@pmnoxx pmnoxx commented Mar 15, 2021

Implement new algorithm for exchanging routing tables using Inverse Bloom Filters.
#3838

Reposting #4108

Copy link
Member

@mfornet mfornet left a comment

Choose a reason for hiding this comment

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

Need to add integration tests that stress different scenarios of synchronization using IBF.

@stale
Copy link

stale bot commented Jul 1, 2021

This PR has been automatically marked as stale because it has not had recent activity in the 2 weeks.
It will be closed in 3 days if no further activity occurs.
Thank you for your contributions.

@stale stale bot added the S-stale label Jul 1, 2021
@bowenwang1996
Copy link
Collaborator

@pmnoxx what is the status here?

@pmnoxx pmnoxx requested review from chefsale and khorolets as code owners July 6, 2021 11:36
@pmnoxx
Copy link
Contributor Author

pmnoxx commented Jul 6, 2021

@pmnoxx what is the status here?

I submitted updated PR. You can take a look.

Copy link
Member

@khorolets khorolets left a comment

Choose a reason for hiding this comment

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

jsonrpc endpoints not follows the way we are handing endpoints. Please, make them as the other endpoints.

@stale
Copy link

stale bot commented Jul 22, 2021

This PR has been automatically marked as stale because it has not had recent activity in the 2 weeks.
It will be closed in 3 days if no further activity occurs.
Thank you for your contributions.

@stale stale bot added the S-stale label Jul 22, 2021
@bowenwang1996
Copy link
Collaborator

@pmnoxx what is the status of this PR?

@stale stale bot removed the S-stale label Jul 22, 2021
@stale
Copy link

stale bot commented Aug 5, 2021

This PR has been automatically marked as stale because it has not had recent activity in the 2 weeks.
It will be closed in 3 days if no further activity occurs.
Thank you for your contributions.

@stale stale bot added S-stale and removed S-stale labels Aug 5, 2021
@stale
Copy link

stale bot commented Aug 23, 2021

This PR has been automatically marked as stale because it has not had recent activity in the 2 weeks.
It will be closed in 3 days if no further activity occurs.
Thank you for your contributions.

@stale stale bot added the S-stale label Aug 23, 2021
@chefsale
Copy link
Contributor

What is the status on this PR? It has again been marked staled? Do we still plan to land this? Can we close it? @pmnoxx @bowenwang1996

@pmnoxx pmnoxx requested a review from frol September 23, 2021 05:44
Copy link
Contributor

@olonho olonho left a comment

Choose a reason for hiding this comment

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

OK for stable hash change.

Copy link
Member

@ailisp ailisp left a comment

Choose a reason for hiding this comment

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

There's not a lot change in runtime, and these changes look okay. As for the IBF in network and chain, I studied and felt confident it's a correct implementation too. I particularly like the unit tests, the benchmark and the pytest, together they prove the correctness and scale in orthogonal ways.

@pmnoxx pmnoxx self-assigned this Sep 23, 2021
@pmnoxx pmnoxx dismissed frol’s stale review September 23, 2021 23:05

Ok, I addressed the issues you mentioned

@pmnoxx
Copy link
Contributor Author

pmnoxx commented Sep 24, 2021

Nayduck passes previously. I ran it one more time after doing all changes:
http://nayduck.near.org/#/run/2082

All tests pass

@pmnoxx
Copy link
Contributor Author

pmnoxx commented Sep 24, 2021

Nayduck passes previously. I ran it one more time after doing all changes:
http://nayduck.near.org/#/run/2082

All tests pass

@frol Is there anything that prevents us from merging?

Copy link
Collaborator

@frol frol left a comment

Choose a reason for hiding this comment

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

I am unblocking this PR from merging, so I am not blocking merging it once all my comments are addressed.

@near-bulldozer near-bulldozer bot merged commit 51ab16a into near:master Sep 25, 2021
near-bulldozer bot pushed a commit that referenced this pull request Sep 28, 2021
…4884)

Fix compilation error in `near-jsonrpc-primitives` when building with the `adversarial` feature enabled after #4112

### Test Plan

- [X] Ensure `near-jsonrpc-primitives` compiles with `adversarial` features turned on
nikurt pushed a commit that referenced this pull request Sep 30, 2021
Implement new algorithm for exchanging routing tables using Inverse Bloom Filters.
#3838

Reposting #4108
near-bulldozer bot pushed a commit that referenced this pull request Oct 4, 2021
…te (#4912)

#4651 successfully removed `near-jsonrpc-primitives`-s heavy dependency on `near-network` which in turn depended on `rocksdb`. #4112 reintroduced that dependency on `near-network`, though optionally, this hinders the publishing process.

This PR extracts the adversarial features that depend on `near-network` into a private, specialized crate: `near-jsonrpc-adversarial-primitives` and sets us right on track for publishing `jsonrpc-primitives`.
@pmnoxx pmnoxx deleted the piotr-new-routing-algorithm-v2 branch November 13, 2021 06:26
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.

10 participants