diff --git a/app/app.go b/app/app.go index 6050299d2..3da14211d 100644 --- a/app/app.go +++ b/app/app.go @@ -109,8 +109,6 @@ type TestConfig struct { TCPNodeCallback func(host.Host) // LibP2POpts provide test specific libp2p options. LibP2POpts []libp2p.Option - // LegacyQBFTProbability defines the probability of a legacy QBFT wire messages (useful for backwards compatibility testing). - LegacyQBFTProbability float64 } // Run is the entrypoint for running a charon DVC instance. @@ -712,7 +710,7 @@ func newConsensus(conf Config, lock cluster.Lock, tcpNode host.Host, p2pKey *k1. } if featureset.Enabled(featureset.QBFTConsensus) { - comp, err := consensus.New(tcpNode, sender, peers, p2pKey, deadliner, qbftSniffer, conf.TestConfig.LegacyQBFTProbability) + comp, err := consensus.New(tcpNode, sender, peers, p2pKey, deadliner, qbftSniffer) if err != nil { return nil, nil, err } diff --git a/app/simnet_test.go b/app/simnet_test.go index eae9f56f5..58c8ee9e6 100644 --- a/app/simnet_test.go +++ b/app/simnet_test.go @@ -297,8 +297,7 @@ func testSimnet(t *testing.T, args simnetArgs, expect simnetExpect) { SimnetBMockOpts: append([]beaconmock.Option{ beaconmock.WithSlotsPerEpoch(1), }, args.BMockOpts...), - BuilderRegistration: registrationFunc(), - LegacyQBFTProbability: 0.5, + BuilderRegistration: registrationFunc(), }, P2P: p2p.Config{}, BuilderAPI: args.BuilderAPI, diff --git a/app/version/version.go b/app/version/version.go index b8e4205f0..104c75bfe 100644 --- a/app/version/version.go +++ b/app/version/version.go @@ -22,7 +22,6 @@ const ( func Supported() []string { return []string{ "v0.14", - "v0.13", } } diff --git a/core/consensus/component.go b/core/consensus/component.go index 3e64764b0..2a35f5a7b 100644 --- a/core/consensus/component.go +++ b/core/consensus/component.go @@ -126,7 +126,7 @@ func newDefinition(nodes int, subs func() []subscriber) qbft.Definition[core.Dut // New returns a new consensus QBFT component. func New(tcpNode host.Host, sender *p2p.Sender, peers []p2p.Peer, p2pKey *k1.PrivateKey, - deadliner core.Deadliner, snifferFunc func(*pbv1.SniffedConsensusInstance), legacyProbability float64, + deadliner core.Deadliner, snifferFunc func(*pbv1.SniffedConsensusInstance), ) (*Component, error) { // Extract peer pubkeys. keys := make(map[int64]*k1.PublicKey) @@ -143,17 +143,16 @@ func New(tcpNode host.Host, sender *p2p.Sender, peers []p2p.Peer, p2pKey *k1.Pri } c := &Component{ - tcpNode: tcpNode, - sender: sender, - peers: peers, - peerLabels: labels, - privkey: p2pKey, - pubkeys: keys, - deadliner: deadliner, - recvBuffers: make(map[core.Duty]chan msg), - snifferFunc: snifferFunc, - dropFilter: log.Filter(), - legacyProbability: legacyProbability, + tcpNode: tcpNode, + sender: sender, + peers: peers, + peerLabels: labels, + privkey: p2pKey, + pubkeys: keys, + deadliner: deadliner, + recvBuffers: make(map[core.Duty]chan msg), + snifferFunc: snifferFunc, + dropFilter: log.Filter(), } c.def = newDefinition(len(peers), c.subscribers) @@ -164,18 +163,17 @@ func New(tcpNode host.Host, sender *p2p.Sender, peers []p2p.Peer, p2pKey *k1.Pri // Component implements core.Consensus. type Component struct { // Immutable state - tcpNode host.Host - sender *p2p.Sender - peerLabels []string - peers []p2p.Peer - pubkeys map[int64]*k1.PublicKey - privkey *k1.PrivateKey - def qbft.Definition[core.Duty, [32]byte] - subs []subscriber - deadliner core.Deadliner - snifferFunc func(*pbv1.SniffedConsensusInstance) - dropFilter z.Field // Filter buffer overflow errors (possible DDoS) - legacyProbability float64 // Probability of using legacy duplicated values inside QBFTMsg vs new pointer values. + tcpNode host.Host + sender *p2p.Sender + peerLabels []string + peers []p2p.Peer + pubkeys map[int64]*k1.PublicKey + privkey *k1.PrivateKey + def qbft.Definition[core.Duty, [32]byte] + subs []subscriber + deadliner core.Deadliner + snifferFunc func(*pbv1.SniffedConsensusInstance) + dropFilter z.Field // Filter buffer overflow errors (possible DDoS) // Mutable state recvMu sync.Mutex diff --git a/core/consensus/component_test.go b/core/consensus/component_test.go index 5d289e0ef..038aa1077 100644 --- a/core/consensus/component_test.go +++ b/core/consensus/component_test.go @@ -86,7 +86,7 @@ func TestComponent(t *testing.T) { sniffed <- len(msgs.Msgs) } - c, err := consensus.New(hosts[i], new(p2p.Sender), peers, p2pkeys[i], testDeadliner{}, sniffer, 0.5) + c, err := consensus.New(hosts[i], new(p2p.Sender), peers, p2pkeys[i], testDeadliner{}, sniffer) require.NoError(t, err) c.Subscribe(func(_ context.Context, _ core.Duty, set core.UnsignedDataSet) error { results <- set diff --git a/core/consensus/msg.go b/core/consensus/msg.go index 68bcaed71..2b543c862 100644 --- a/core/consensus/msg.go +++ b/core/consensus/msg.go @@ -23,34 +23,14 @@ func newMsg(pbMsg *pbv1.QBFTMsg, justification []*pbv1.QBFTMsg, values map[[32]b preparedValueHash [32]byte ) - if pbMsg.Value != nil { // Use legacy value inside QBFTMsg. - value, err := pbMsg.Value.UnmarshalNew() - if err != nil { - return msg{}, errors.Wrap(err, "unmarshal any") - } - valueHash, err = hashProto(value) - if err != nil { - return msg{}, err - } - values[valueHash] = pbMsg.Value - } else if hash, ok := toHash32(pbMsg.ValueHash); ok { + if hash, ok := toHash32(pbMsg.ValueHash); ok { valueHash = hash if _, ok := values[valueHash]; !ok { return msg{}, errors.New("value hash not found in values") } } - if pbMsg.PreparedValue != nil { // Use legacy prepared value inside QBFTMsg. - pv, err := pbMsg.PreparedValue.UnmarshalNew() - if err != nil { - return msg{}, errors.Wrap(err, "unmarshal any") - } - preparedValueHash, err = hashProto(pv) - if err != nil { - return msg{}, err - } - values[valueHash] = pbMsg.PreparedValue - } else if hash, ok := toHash32(pbMsg.PreparedValueHash); ok { + if hash, ok := toHash32(pbMsg.PreparedValueHash); ok { preparedValueHash = hash if _, ok := values[preparedValueHash]; !ok { return msg{}, errors.New("prepared value hash not found in values") diff --git a/core/consensus/msg_internal_test.go b/core/consensus/msg_internal_test.go index 06fb4fd3d..f6ab59686 100644 --- a/core/consensus/msg_internal_test.go +++ b/core/consensus/msg_internal_test.go @@ -108,7 +108,7 @@ func TestPartialLegacyNewMsg(t *testing.T) { ValueHash: hash1[:], }, }, make(map[[32]byte]*anypb.Any)) - require.NoError(t, err) + require.ErrorContains(t, err, "value hash not found in values") } // randomMsg returns a random qbft message. diff --git a/core/consensus/transport.go b/core/consensus/transport.go index 0757d6c66..59af80eef 100644 --- a/core/consensus/transport.go +++ b/core/consensus/transport.go @@ -4,7 +4,6 @@ package consensus import ( "context" - "crypto/rand" "sync" "time" @@ -55,45 +54,42 @@ func (t *transport) getValue(hash [32]byte) (*anypb.Any, error) { return pb, nil } -// usePointerValues returns true if the transport should use pointer values in the message instead of the legacy -// duplicated values in QBFTMsg. -func (t *transport) usePointerValues() bool { - // Equivalent to math/rand.Float64() just with less precision. - b := make([]byte, 1) - _, _ = rand.Read(b) - f := float64(b[0]) / 255 - - return f >= t.component.legacyProbability -} - // Broadcast creates a msg and sends it to all peers (including self). func (t *transport) Broadcast(ctx context.Context, typ qbft.MsgType, duty core.Duty, peerIdx int64, round int64, valueHash [32]byte, pr int64, pvHash [32]byte, justification []qbft.Msg[core.Duty, [32]byte], ) error { - // Get the values by their hashes if not zero. - var ( - value *anypb.Any - pv *anypb.Any - err error - ) - - if valueHash != [32]byte{} { - value, err = t.getValue(valueHash) - if err != nil { - return err + // Get all hashes + var hashes [][32]byte + hashes = append(hashes, valueHash) + hashes = append(hashes, pvHash) + for _, just := range justification { + msg, ok := just.(msg) + if !ok { + return errors.New("invalid justification message") } + hashes = append(hashes, msg.valueHash) + hashes = append(hashes, msg.preparedValueHash) } - if pvHash != [32]byte{} { - pv, err = t.getValue(pvHash) + // Get values by their hashes if not zero. + values := make(map[[32]byte]*anypb.Any) + for _, hash := range hashes { + if hash == [32]byte{} || values[hash] != nil { + continue + } + + value, err := t.getValue(hash) if err != nil { return err } + + values[hash] = value } // Make the message - msg, err := createMsg(typ, duty, peerIdx, round, valueHash, value, pr, pvHash, pv, justification, t.component.privkey, t.usePointerValues()) + msg, err := createMsg(typ, duty, peerIdx, round, valueHash, pr, + pvHash, values, justification, t.component.privkey) if err != nil { return err } @@ -152,37 +148,17 @@ func (t *transport) ProcessReceives(ctx context.Context, outerBuffer chan msg) { // createMsg returns a new message by converting the inputs into a protobuf // and wrapping that in a msg type. func createMsg(typ qbft.MsgType, duty core.Duty, - peerIdx int64, round int64, - vHash [32]byte, value *anypb.Any, - pr int64, - pvHash [32]byte, pv *anypb.Any, - justification []qbft.Msg[core.Duty, [32]byte], privkey *k1.PrivateKey, - pointerValues bool, + peerIdx int64, round int64, vHash [32]byte, pr int64, pvHash [32]byte, + values map[[32]byte]*anypb.Any, justification []qbft.Msg[core.Duty, [32]byte], + privkey *k1.PrivateKey, ) (msg, error) { - values := make(map[[32]byte]*anypb.Any) - if value != nil { - values[vHash] = value - } - if pv != nil { - values[pvHash] = pv - } - - // Disable new pointer values, revert to legacy duplicated values. - if !pointerValues { - values = make(map[[32]byte]*anypb.Any) - vHash = [32]byte{} - pvHash = [32]byte{} - } - pbMsg := &pbv1.QBFTMsg{ Type: int64(typ), Duty: core.DutyToProto(duty), PeerIdx: peerIdx, Round: round, - Value: value, ValueHash: vHash[:], PreparedRound: pr, - PreparedValue: pv, PreparedValueHash: pvHash[:], }