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

A0-3364: cache network details of validators #1449

Merged
merged 13 commits into from
Oct 25, 2023
Merged

A0-3364: cache network details of validators #1449

merged 13 commits into from
Oct 25, 2023

Conversation

ggawryal
Copy link
Contributor

@ggawryal ggawryal commented Oct 20, 2023 •
edited
Loading

Description

Adding structures that will be later helpful to construct RPC call to get network details of validators. This is a part 2/3 of the whole task, see #1447). This only pulls and caches network data that was previously observed by a node, without modifying or adding any new information flow between nodes in the network.

Follow up to #1448

Type of change

Please delete options that are not relevant.

  • New feature (non-breaking change which adds functionality)

Checklist:

  • I have added tests
  • I have created new documentation

@ggawryal ggawryal mentioned this pull request Oct 20, 2023
Merged
@ggawryal
Copy link
Contributor Author

ggawryal commented Oct 20, 2023 •
edited
Loading

Left ValidatorAddressCache to be None by default, which will remove almost whole overhead from addressing cache updates. Particularly, there is no runtime call to read key owner from db. However, we still compute SignedTcpAddressingInformation.lower_level_address, but I assume this is acceptable?

@ggawryal ggawryal marked this pull request as ready for review October 20, 2023 13:01
kostekIV
kostekIV reviewed Oct 20, 2023
View reviewed changes
Copy link
Contributor

@kostekIV kostekIV left a comment

Choose a reason for hiding this comment

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

Looks great, one last comment

finality-aleph/src/network/session/manager.rs Show resolved Hide resolved
timorleph
timorleph requested changes Oct 23, 2023
View reviewed changes
Copy link
Contributor

@timorleph timorleph left a comment

Choose a reason for hiding this comment

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

Cool stuff, some comments, although many of them ended up being almost one comment written in multiple places. :P

clique/src/lib.rs Outdated Show resolved Hide resolved
finality-aleph/src/network/tcp.rs Outdated Show resolved Hide resolved
finality-aleph/src/network/address_cache.rs Outdated Show resolved Hide resolved
finality-aleph/src/network/address_cache.rs Outdated Show resolved Hide resolved
finality-aleph/src/network/address_cache.rs Outdated Show resolved Hide resolved
finality-aleph/src/network/address_cache.rs Outdated Show resolved Hide resolved
bin/node/src/service.rs Show resolved Hide resolved
timorleph
timorleph approved these changes Oct 24, 2023
View reviewed changes
Copy link
Contributor

@timorleph timorleph left a comment

Choose a reason for hiding this comment

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

Minor comments left.

finality-aleph/src/network/address_cache.rs Show resolved Hide resolved
finality-aleph/src/network/address_cache.rs Outdated Show resolved Hide resolved
@ggawryal ggawryal added this pull request to the merge queue Oct 25, 2023
Merged via the queue into Cardinal-Cryptography:main with commit 9bd08cf Oct 25, 2023
9 checks passed
@ggawryal ggawryal deleted the a0-3364-cache-network-details-of-validators branch October 25, 2023 08:44
github-merge-queue bot pushed a commit that referenced this pull request Nov 9, 2023
@ggawryal
A0-3094: Validators network details rpc call (#1447)
97bc9cf 
# Description

Adding an RPC call to get network details of validators. This only pulls
and caches network data that was previously observed by a node, without
modifying or adding any new information flow between nodes in the
network.

Related PRs: #1448 #1449

<details>
  <summary> RPC query output on local network </summary>
  
```curl -H "Content-Type: application/json" -d '{"id":1,
"jsonrpc":"2.0", "method": "alephNode_unstable_validatorNetworkInfo",
"params": []}' http://localhost:9948/```
  ```
 {
    "jsonrpc": "2.0",
    "result": {
        "5F4H97f7nQovyrbiq4ZetaaviNwThSVcFobcA5aGab6167dK": {
            "session": 1,
            "network_level_address": "127.0.0.1:30346",
"validator_network_peer_id":
"5CwWuuXMCvcSC7hqeMdFSfNFzVGXCAtdSGAMjwV1nLwaqtqE"
        },
        "5GBNeWRhZc2jXu7D55rBimKYDk8PGk8itRYFTPfC8RJLKG5o": {
            "session": 1,
            "network_level_address": "127.0.0.1:30344",
"validator_network_peer_id":
"5H7yZo8EzYkSsZn7MgC4f9vaVYQLgNaS4kEZ29Q7WtiYMVF8"
        },
        "5Dfis6XL8J2P6JHUnUtArnFWndn62SydeP8ee8sG2ky9nfm9": {
            "session": 1,
            "network_level_address": "127.0.0.1:30345",
"validator_network_peer_id":
"5HZFGbT6tsY2iV2YT927wnN4q2TxmPT67SUJE99TqGQsjpGV"
        },
        "5D34dL5prEUaGNQtPPZ3yN5Y6BnkfXunKXXz6fo7ZJbLwRRH": {
            "session": 1,
            "network_level_address": "127.0.0.1:30343",
"validator_network_peer_id":
"5E1YcJyx4u3kQ4UiRPgPXPSvb2vbcCR2DsxS927GM6x8wz85"
        }
    },
    "id": 1
}                        
  ```

</details>

Has been run also on testnet, slowly collects network details for
authorities in each session (without `AccountId` because of the old
runtime version).

# Type of change

Please delete options that are not relevant.

- New feature (non-breaking change which adds functionality)

# Checklist:

<!-- delete when not applicable to your PR -->

- I have created new documentation
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@timorleph timorleph timorleph approved these changes

@kostekIV kostekIV kostekIV approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ggawryal @kostekIV @timorleph