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

feat: add request-response #14

Merged
merged 4 commits into from
Jan 13, 2025
Merged

Conversation

NikitaMasych
Copy link
Collaborator

@NikitaMasych NikitaMasych commented Jan 10, 2025

Description

Add request-response protocol.

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature/Enhancement (non-breaking change which adds functionality or enhances an existing one)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Refactor
  • New or updated tests
  • Dependency Update

Notes to Reviewers

Basic, untested version.

Checklist

  • I have performed a self-review of my code.
  • I have commented my code where necessary.
  • I have updated the documentation if needed.
  • My changes do not introduce new warnings.
  • I have added tests that prove my changes are effective or that my feature works.
  • New and existing tests pass with my changes.

Related Issues

@NikitaMasych NikitaMasych self-assigned this Jan 10, 2025
Copy link

github-actions bot commented Jan 10, 2025

The latest Buf updates on your PR. Results from workflow Lint / proto (pull_request).

BuildFormatLintBreakingUpdated (UTC)
✅ passed✅ passed✅ passed✅ passedJan 13, 2025, 4:41 PM

@NikitaMasych NikitaMasych marked this pull request as draft January 10, 2025 16:26
@NikitaMasych NikitaMasych marked this pull request as ready for review January 13, 2025 12:39
@NikitaMasych NikitaMasych requested a review from Velnbur January 13, 2025 12:39
crates/db/src/lib.rs Outdated Show resolved Hide resolved
crates/p2p/src/swarm/mod.rs Outdated Show resolved Hide resolved
crates/p2p/src/swarm/mod.rs Show resolved Hide resolved
crates/p2p/src/swarm/mod.rs Show resolved Hide resolved
crates/p2p/src/swarm/mod.rs Outdated Show resolved Hide resolved
crates/p2p/src/swarm/mod.rs Outdated Show resolved Hide resolved
crates/p2p/src/swarm/mod.rs Outdated Show resolved Hide resolved
} => {
if response.msg.is_empty() {
debug!(%request_id, "Have no needed data, requesting from neighbours");
return Ok(()); // TODO(NikitaMasych): launch recursive request.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think the node should request this data again after some time instead, so we may have TimeoutManager to be an event-loop, reactor kind of thing instead of a place for timeouts only. But let's consider this in next PR

@NikitaMasych NikitaMasych requested a review from Velnbur January 13, 2025 15:04
@NikitaMasych NikitaMasych requested a review from Velnbur January 13, 2025 16:42
@NikitaMasych NikitaMasych merged commit 10f519a into feat/basic-swarm Jan 13, 2025
12 checks passed
@NikitaMasych NikitaMasych deleted the feat/request-response branch January 13, 2025 16:51
storopoli pushed a commit that referenced this pull request Jan 28, 2025
* feat: basic swarm

* Make swarm and database constructable from outside of P2P type (#9)

This is needed for P2P users to specify `memory` or `tcp` transport
and whether the database should be temporarly, for tests and
production accordingly.

Co-authored-by: Velnbur <[email protected]>

* Move db and protobuf related code to separate crates (#8)

Move `crates/p2p/src/db` to `crates/db`, `crates/p2p/src/wire` to
`crates/wire` and then `crates/wire/proto/` out of `wire` to top level
`proto`. Also some minor refactor stuff and fixes to make CI happy
were applied.

* chore: fix ci (#11)

* chore: moved nextest config to .config/

* chore: add github token to arduino/setup-protoc

* Move `tokio`, `tokio-util` and `tracing-subscriber` to workspace deps (#10)

* feat: Add new methods for tests, make `init` public and add test (#12)

`init` was renamed to `establish_connections` and made public, so the crate user can control stages of P2P readiness.

Added test with sequential handling of multiple scopes with multiple operators.

* Add doc comments to some of the methods and remove redundant code (#13)

* feat: add request-response (#14)

* feat: added basic request-response

* feat: added pub key storage in db entry and signature validation

* fix: review issues resolved

* fix: refactor pubkey serde + remove all_messages_empty

* Add async threadpooled wrapper for `sled::Db` (#16)

* refactor: operator id not via peer id (#15)

* refactor: operator id now independent of peer id

* fix: post-review improvements

* chore: taplo fmt

* fix: resolve end-to-end tests

* chore: cargo fmt --all

* chore: taplo fmt

* chore: remove unused deps in cargo.toml files

* draft: added whitelist of signers

* fix: resolve issues

* refactor: move validation into p2p

* fix: resolve some review comments

* fix: resolve db test private field

* refactor: migrated from snafu to thiserror (#17)

* refactor: migrated from snafu to thiserror

* refactor: post-review improvements

* refactor: post-review improvements

* fix(db): agnostic InvalidData error

* fix

* Remove `TimeoutManager` and add command for requesting messages for `P2PHandle` (#18)

* Remove timeout manager

To simplify current code for Testnet I, as state handling of peers is
not in the scope for now.

* Add implementation of direct `RequestMessage` command for handles

By removing timeouts handling in the previous commit, we didn't left
an option for P2P users to request something through
`request-response` behaviour. That's why this commit adds handling of
`Command::RequestMessage`.

Added persistent mapping of operator pubkey to peer id, that
distributed messages from this operators, so P2P can request that
message directly by retrieving peer ID from this mapping.

* Make threadpool for DB with one core for tests instead of all

* refactor: request command use fallback neighbours request (#20)

* refactor: added request from neighbours in request command

* wip: added request-response test

* fix: resolved test

* refactor: upd tests

* trying to fix compiler bug

* fix visibility in tests

* refactor: renamed OperatorLever

* refactor: moved handling of request-response message to separate function

* refactor: Use `SessionId` instead of `Scope` for nonces and signatures exchange messages (#19)

* Introduce `SessionId` and `Scope` types

* Replace hash with scope, refactor names

* Add `CleanStorage` command for P2P (#21)

`CleanStorageCommand` - deletes all entries from storage by session
IDs, scopes and operators' pubkeys.

For that `delete_raw` method for `Repository` was added, which deletes
entries at once by array of keys.

Implementation creates cartesian products from sets of operators'
pubkeys and session IDs, operators' pubkeys and scopes, then
deletes all some entries at once, but sequantially for each type
of entry (nonces, signatures or deposit setups).

---------

Co-authored-by: Velnbur <[email protected]>
Co-authored-by: Kyrylo Baibula <[email protected]>
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