Add ValidatorInfo to PeerContacts. by nibhar · Pull Request #3039 · nimiq/core-rs-albatross

nibhar · 2024-11-12T01:52:01Z

This PR adds Validators information to the PeerContacts exchanged during discovery. In this PR the information is not populated, neither is it utilized. It is intended as an additional means of retrieving validator information aside from just the dht.

In addition to that it changes how peer contact information gets discarded in case it is faulty. Not discarding everything, but only the faulty piece of data instead. As Validator contacts can only be verified when the state is complete, and only if the respective key has not been rotated out (or in) yet, an entry in the address book can be flagged as local_only in those cases where verification is not (yet) possible. They should be re-evaluated at a time, when the verification is possible. Local_only entries will not be put into discovery messages.

jsdanielh · 2024-11-25T21:43:42Z

+        if let Some(validator_info) = &info.contact.inner.validator_info {
+            self.validator_peer_ids
+                .entry(validator_info.validator_address.clone())
+                .or_insert(HashSet::new())


Suggested change

.or_insert(HashSet::new())

.or_default()

jsdanielh · 2024-11-25T21:45:39Z

+        contact.validator_info = self.validator_record_signing.as_ref().and_then(|callback| {
+            let tagged_signed = (callback)(contact.peer_id(), contact.timestamp);
+            Some(ValidatorInfo {
+                validator_address: tagged_signed.record.validator_address.clone(),
+                signature: tagged_signed.signature.clone(),
+            })


Suggested change

contact.validator_info = self.validator_record_signing.as_ref().and_then(|callback| {

let tagged_signed = (callback)(contact.peer_id(), contact.timestamp);

Some(ValidatorInfo {

validator_address: tagged_signed.record.validator_address.clone(),

signature: tagged_signed.signature.clone(),

})

contact.validator_info = self.validator_record_signing.as_ref().map(|callback| {

let tagged_signed = (callback)(contact.peer_id(), contact.timestamp);

ValidatorInfo {

validator_address: tagged_signed.record.validator_address.clone(),

signature: tagged_signed.signature.clone(),

}

jsdanielh · 2024-11-25T23:38:47Z

    /// Timer to do house-keeping in the peer address book.
    house_keeping_timer: Interval,
+
+    /// dht verifier TODO


What is this TODO?

jsdanielh · 2024-11-25T23:40:10Z

-                connection: CloseConnection::All,
-            }),
+            // Errors must not result in a closed connection as light clients are unable to verify ValidatorRecord.
+            HandlerOutEvent::Error(error) => log::trace!(?error, "Received invalid contact"),


Mmm, but we panic if a light client tries to verify a validator record 🤔

jsdanielh · 2024-11-25T23:47:24Z

+                                    let peer_contacts: Vec<SignedPeerContact> = peer_contacts
+                                        .into_iter()
+                                        .filter_map(filter_contact(
+                                            0,


jsdanielh · 2024-11-25T23:47:32Z

+                                    let peer_contacts: Vec<SignedPeerContact> = peer_contacts
+                                        .into_iter()
+                                        .filter_map(filter_contact(
+                                            0,


jsdanielh · 2024-11-25T23:47:40Z

                                    // Check the peer contact for a valid signature.
-                                    if !peer_contact.verify() {
+                                    let Some(peer_contact) = filter_contact(
+                                        0,


ii-cruz · 2024-11-25T22:37:29Z

+        }
+
+        let results = futures.collect::<Vec<Result<(), RequestError>>>().await;
+        if results.iter().any(|result| result.is_ok()) {


what about clearing the peers that didn't work?

ii-cruz · 2024-11-25T23:50:46Z

    }
+
+    fn get_peers_by_validator(&self, _validator_address: &Address) -> Vec<Self::PeerId> {
+        // TODO


Suggested change

// TODO

unimplemented!()

ii-cruz · 2024-11-25T23:51:09Z

+        &self,
+        _callback: impl Fn(Self::PeerId, u64) -> TaggedSigned<ValidatorRecord<Self::PeerId>, KeyPair>,
+    ) {
+        // TODO


Suggested change

// TODO

unimplemented!()

ii-cruz · 2024-11-25T23:52:01Z

    /// Timer to do house-keeping in the peer address book.
    house_keeping_timer: Interval,
+
+    /// dht verifier TODO


Suggested change

/// dht verifier TODO

/// DHT validator record verifier.

ii-cruz

This PR is failing on the Build crate features. Haven't looked at it yet.

…m the contact book

…book

nibhar force-pushed the nibhar/validator-discovery branch 4 times, most recently from bac56d8 to 2c0c433 Compare November 12, 2024 03:14

nibhar force-pushed the nibhar/validator-discovery branch from 2c0c433 to 4d66caa Compare November 25, 2024 06:51

ii-cruz force-pushed the nibhar/validator-discovery branch 2 times, most recently from aad39ad to 145c075 Compare November 25, 2024 09:20

nibhar force-pushed the nibhar/validator-discovery branch from d1915ac to afaa762 Compare November 25, 2024 10:58

paberr reviewed Nov 25, 2024

View reviewed changes

Comment thread validator-network/src/network_impl.rs

Comment thread network-libp2p/src/discovery/peer_contacts.rs Outdated

nibhar force-pushed the nibhar/validator-discovery branch from 42e4099 to 10e9372 Compare November 25, 2024 18:07

nibhar marked this pull request as ready for review November 25, 2024 18:28

jsdanielh reviewed Nov 25, 2024

View reviewed changes

ii-cruz reviewed Nov 25, 2024

View reviewed changes

ii-cruz reviewed Nov 26, 2024

View reviewed changes

nibhar and others added 16 commits January 22, 2025 14:41

Add ValidatorInfo to the PeerContacts. Do basic verification

80fce1a

Compile fix

a07c03b

Add/remove peer ids from validator addresses

98e0d1f

Network: prepare network interface for validator discovery

ab3fc09

Populate validator_info

63403bf

ValidatorNetwork: resolve validator peer ids from peer contact book

20dc107

Validator: register callback for the validator record

802d32a

DhtVerifier: Remove unused and commented code

51136f0

DHT: fix clippy warnings

833bdda

ValidatorNetwork: use correct validator_id when fetching peer ids fro…

8f7fe54

…m the contact book

Network: fix filtering of local_only contacts

c867f84

Fix clippy warnings

4586742

Fixing the feature compilation

e592e10

ValidatorNetwork: use contact book peer ids where required

3058be2

Validator: register signing callback in constructor

09e6a5d

Network: correctly update own peer contact

a84c105

styppo force-pushed the nibhar/validator-discovery branch from 2722eee to a84c105 Compare January 22, 2025 15:07

styppo added 3 commits January 22, 2025 15:07

ValidatorNetwork: cache verified validator peer ids from the contact …

9b8c194

…book

Network: periodically re-verify local_only contacts in peer contact book

c94fdb5

Network: close connection for all discovery errors

bb0a8b7

styppo force-pushed the nibhar/validator-discovery branch from 14a7780 to 7fdc909 Compare January 30, 2025 23:18

Network: fix wasm compilation error

7b39268

styppo force-pushed the nibhar/validator-discovery branch from 7fdc909 to 7b39268 Compare January 31, 2025 14:24

Uh oh!

Conversation

nibhar commented Nov 12, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ii-cruz left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants