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

Process inv with mixed item types #1768

Closed
7 tasks
teor2345 opened this issue Feb 17, 2021 · 1 comment
Closed
7 tasks

Process inv with mixed item types #1768

teor2345 opened this issue Feb 17, 2021 · 1 comment
Labels
A-rust Area: Updates to Rust code C-bug Category: This is a bug C-cleanup Category: This is a cleanup C-enhancement Category: This is an improvement

Comments

@teor2345
Copy link
Contributor

teor2345 commented Feb 17, 2021

Work In Progress

Is your feature request related to a problem?

Some of my mainnet and testnet Zebra instances occasionally log messages like:

Feb 18 07:47:55.715 WARN {zebrad="12cb8492" net="Main"}:sync: zebrad::components::sync: error downloading and verifying block e=SharedPeerError(WrongMessage("inv with mixed item types"))

Describe the solution you'd like

Process inventory (inv) messages with mixed item types by splitting them into an internal Zebra Response for each type of item:

  • split messages in the peer_rx stream in Handshake, before the inventory collector runs on the stream
  • split Inv messages by InventoryHash type
  • split small multi-block Inv messages into a series of single block messages

Cleanup the handshake code (this should be a separate PR):

  • move the large peer_rx closures into separate functions
  • move connection wrapper closures into functions
  • apply all connection wrappers in handshake, and move them to that file
    • make a comment in connection explaining that Handshake setup also changes connection messages

This will be easier when we upgrade to the latest tower and tokio (#2200), because they implement more trait bounds like Sync and Send.

Describe alternatives you've considered

  1. Continue to ignore inv with mixed item types, and continue logging warnings
  2. Log an info-level message instead of a warning
  3. Add more debugging to work out what the item types actually are

Additional context

I don't know if we need these messages for normal operation - we might be missing out on some gossiped blocks or transactions, or we might be missing out on some responses that were merged into the same inv message.

This is a low priority, until it actually affects Zebra, or the messages become much more frequent.

@teor2345 teor2345 added C-bug Category: This is a bug A-rust Area: Updates to Rust code C-enhancement Category: This is an improvement C-cleanup Category: This is a cleanup S-needs-triage Status: A bug report needs triage P-Low labels Feb 17, 2021
@teor2345 teor2345 added this to No Estimate in Effort Affinity Grouping via automation Feb 17, 2021
@teor2345 teor2345 changed the title Allow inv with mixed item types Process inv with mixed item types Feb 17, 2021
@teor2345 teor2345 added this to To Do in 🦓 via automation Feb 17, 2021
@teor2345 teor2345 moved this from No Estimate to XS - 2 in Effort Affinity Grouping Feb 17, 2021
@mpguerra mpguerra added P-Medium and removed S-needs-triage Status: A bug report needs triage P-Low labels Feb 22, 2021
@teor2345
Copy link
Contributor Author

Obsoleted by #2156 - if we miss any inv items, the notfound response will stop us trying that peer again.

🦓 automation moved this from To Do to Done Jan 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-rust Area: Updates to Rust code C-bug Category: This is a bug C-cleanup Category: This is a cleanup C-enhancement Category: This is an improvement
Projects
No open projects
Development

No branches or pull requests

2 participants