Improve crosslink on devnet #4669
Conversation
Frozen
force-pushed
the
fix/crosslink-loop
branch
2 times, most recently
from
72b2c6b
to
db9fe4b
Compare
Frozen
force-pushed
the
fix/crosslink-loop
branch
from
098e561
to
868c897
Compare
| go func() { | ||
| for { | ||
| <-time.After(10 * time.Second) | ||
| go currentNode.BroadcastCrossLinkFromShardsToBeacon() |
There was a problem hiding this comment.
why do we need this here ?
There was a problem hiding this comment.
BroadcastCrossLinkFromShardsToBeacon happens only on a new block insertion. There is infinity view change, no new blocks.
There was a problem hiding this comment.
It's not always the case that we are in an infinite view change. Under normal conditions (i.e., no view change), could there be any race conditions here, or what happens if we broadcasting a crosslink twice?
Question is why would a shard need to broadcast a crosslink when it is not producing block?
There was a problem hiding this comment.
what happens if we broadcasting a crosslink twice
crosslink broadcasted by leader and 1% of validators. It's duplicated by design.
why would a shard need to broadcast a crosslink when it is not producing block
Because shard 0 broadcasts struct CrosslinkHeartbeat with field LatestContinuousBlockNum. It knows about last blocks received by crosslink, but crosslink should be sent first. Actually, that is the second issue, where we can't verify CrosslinkHeartbeat message, because no key in committee, which is solved here
so, there is circular dependency, no heartbeat because no crosslinks, but no crosslinks because no heartbeat.
| utils.Logger().Error().Err(err).Msg("[BroadcastCrossLinkSignal] failed to get committee") | ||
| continue | ||
| } | ||
| privToSing := getPrivToSign(node.Consensus.GetPrivateKeys(), comm) |
There was a problem hiding this comment.
shouldn't we make sure to use the leader private key to sign instead of any private key available on the node?
| q := createVoters(comm) | ||
| for _, priv := range privs { | ||
| _, found := q[priv.Pub.Bytes] | ||
| if found { // if key is in committee |
There was a problem hiding this comment.
depending on the use case of getPrivToSign, there might be instances where we need the leader key instead of any keys currently elected in the committee
Improved crosslink signing