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

kad/store: Set memory-store on an incoming record for PutRecordTo #88

Merged
merged 3 commits into from
Apr 24, 2024

Conversation

lexnv
Copy link
Collaborator

@lexnv lexnv commented Apr 24, 2024

Small PR to always update the local store on PutRecordToPeers.

This is introduced to facilitate the use-case from:

Where the update_local_store flag is used to optimize kad MemoryStore updates. However, for us this represents a hashmap insert.

While at it, have also filtered out peers that are equal to our local peer ID.

cc @dmitry-markin

@lexnv lexnv self-assigned this Apr 24, 2024
@dmitry-markin
Copy link
Collaborator

It looks like this is different to what libp2p does. May be keep the operations consistent between libraries until we keep only one of them and handle this in the client code?

@dmitry-markin
Copy link
Collaborator

The idea behind put_record_to is to update the peers with outdated records, even in cases where per kademlia metric we should not store the record locally. To keep this possible, we can update the PR introducing a flag for optional local store update used instead of direct store accesses like in libp2p.

Signed-off-by: Alexandru Vasile <[email protected]>
Copy link
Collaborator

@dmitry-markin dmitry-markin left a comment

Choose a reason for hiding this comment

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

LGTM!

@lexnv lexnv merged commit 67b2764 into master Apr 24, 2024
7 checks passed
@lexnv lexnv deleted the lexnv/store-update branch April 24, 2024 15:47
@lexnv lexnv mentioned this pull request May 24, 2024
lexnv added a commit that referenced this pull request May 24, 2024
## [0.5.0] - 2023-05-24

This is a small patch release that makes the `FindNode` command a bit
more robst:

- The `FindNode` command now retains the K (replication factor) best
results.
- The `FindNode` command has been updated to handle errors and
unexpected states without panicking.

### Changed

- kad: Refactor FindNode query, keep K best results and add tests
([#114](#114))

## [0.4.0] - 2023-05-23

This release introduces breaking changes to the litep2p crate, primarily
affecting the `kad` module. Key updates include:

- The `GetRecord` command now exposes all peer records, not just the
latest one.
- A new `RecordType` has been introduced to clearly distinguish between
locally stored records and those discovered from the network.

Significant refactoring has been done to enhance the efficiency and
accuracy of the `kad` module. The updates are as follows:

- The `GetRecord` command now exposes all peer records.
- The `GetRecord` command has been updated to handle errors and
unexpected states without panicking.

Additionally, we've improved code coverage in the `kad` module by adding
more tests.

### Added

- Add release checklist
([#115](#115))
- Re-export `multihash` & `multiaddr` types
([#79](#79))
- kad: Expose all peer records of `GET_VALUE` query
([#96](#96))

### Changed

- multistream_select: Remove unneeded changelog.md
([#116](#116))
- kad: Refactor `GetRecord` query and add tests
([#97](#97))
- kad/store: Set memory-store on an incoming record for PutRecordTo
([#88](#88))
- multistream: Dialer deny multiple /multistream/1.0.0 headers
([#61](#61))
- kad: Limit MemoryStore entries
([#78](#78))
- Refactor WebRTC code
([#51](#51))
- Revert "Bring `rustfmt.toml` in sync with polkadot-sdk (#71)"
([#74](#74))
- cargo: Update str0m from 0.4.1 to 0.5.1
([#95](#95))

### Fixed

- Fix clippy  ([#83](#83))
- crypto: Don't panic on unsupported key types
([#84](#84))

---------

Signed-off-by: Alexandru Vasile <[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