Page MenuHomePhabricator

[avalanche] enforce at least one delegation level in avahello
AbandonedPublic

Authored by PiRK on May 11 2021, 10:41.

Details

Reviewers
deadalnix
Group Reviewers
Restricted Project
Summary

Previously we didn't enforce that delegations sent in the avahello message have at least one delegation level. As a result, when receiving and empty delegation when we don't already have the proof, we could'nt immediately check the avahello signature because we don't know the public key.

This ensure that a node always generates a delegation with a level. The only case in which this didn't already happen was if a user specified an identical -avasessionkey and -avamasterkey. Now in such a case we just add a level which delegates to the same key, which is a perfectly valid delegation.

Empty delegations are still considered valid, and can happen when a node is added via the addavalanchenode, in which case we always have the proof to retrieve the master key from.

This also adds a Delegation::getMaster method, which return the pubkey of the last level of the delegation, and uses it to check the delegation is not empty when receiving an AVAHELLO message.

Functional tests are added to check the node's behavior when receiving an empty delegation or an invalid signature.

Test Plan

ninja all check-all

Diff Detail

Event Timeline

PiRK requested review of this revision.May 11 2021, 10:41

Add missing return; when done with checking the AVAHELLO message.
Add a log message for successful signature verification.

deadalnix requested changes to this revision.May 11 2021, 22:19
deadalnix added a subscriber: deadalnix.

Either delegation must have at least one level or they don't. It doesn't make sense to insist on avahello having some special type of delegation.

This revision now requires changes to proceed.May 11 2021, 22:19

I'm going to keep looking for solutions, but it is not as simple as I thought. There are methods (e.g. PeerManager::addNode that take a delegation even when it is not strictly necessary, and currently we just build a dummy delegation for this case. So it will either make the delegation mandatory always (or at least specifying a private key to build a delegation locally), even for adding a node via RPC, or we need to make the Delegation an optional parameter for this method.