Remove the remaining mechanism for swappable payload marshal for signing (#927)

Previous versions of go-f3 made marshal for signing swappable during
tests to help tests run faster. This has caused much confusion in
debugging false negative test failures.

As a result, the fake marshal for signing was changed to use a format
consistent with the real network (#923).

As a result we no longer need the types and mechanisms in code to make
marshalling of payload for signing swappable. Remove them all.

Author: masih

certchain/certchain.go

@@ -165,7 +165,7 @@ func (cc *CertChain) signProportionally(ctx context.Context, committee *gpbft.Co
 		signature   []byte
 	}
 	var signatures []signatureAt
-	marshalledPayload := cc.sv.MarshalPayloadForSigning(cc.m.NetworkName, payload)
+	marshalledPayload := payload.MarshalForSigning(cc.m.NetworkName)
 	for _, p := range candidateSigners {
 		scaledPower, key := committee.PowerTable.Get(p.ID)
 		if scaledPower == 0 {
@@ -220,7 +220,7 @@ func (cc *CertChain) sign(ctx context.Context, committee *gpbft.Committee, paylo
 	var signingPowerSoFar int64
 	var signatures [][]byte
 	var signersMask []int
-	marshalledPayload := cc.sv.MarshalPayloadForSigning(cc.m.NetworkName, payload)
+	marshalledPayload := payload.MarshalForSigning(cc.m.NetworkName)
 	if err := signers.ForEach(
 		func(signerIndex uint64) error {
 			p := committee.PowerTable.Entries[signerIndex]

certchain/options.go

@@ -13,7 +13,6 @@ type Option func(*options) error
 type SignVerifier interface {
 	gpbft.Signer
 	gpbft.Verifier
-	gpbft.SigningMarshaler
 }
 
 type options struct {

certs/certs.go

@@ -184,20 +184,11 @@ func verifyFinalityCertificateSignature(verifier gpbft.Verifier, powerTable gpbf
 		Value:            cert.ECChain,
 	}
 
-	// We use SigningMarshaler when implemented (for testing), but only require a `Verifier` in
-	// the function signature to make it easier to use this as a free function.
-	var signedBytes []byte
-	if sig, ok := verifier.(gpbft.SigningMarshaler); ok {
-		signedBytes = sig.MarshalPayloadForSigning(nn, payload)
-	} else {
-		signedBytes = payload.MarshalForSigning(nn)
-	}
-
 	aggregate, err := verifier.Aggregate(keys)
 	if err != nil {
 		return err
 	}
-
+	signedBytes := payload.MarshalForSigning(nn)
 	if err := aggregate.VerifyAggregate(mask, signedBytes, cert.Signature); err != nil {
 		return fmt.Errorf("invalid signature on finality certificate for instance %d: %w", cert.GPBFTInstance, err)
 	}

emulator/driver.go

@@ -88,7 +88,6 @@ func (d *Driver) prepareMessage(partialMessage *gpbft.GMessage) *gpbft.GMessage
 
 	mb := instance.NewMessageBuilder(partialMessage.Vote, partialMessage.Justification, withValidTicket)
 	mb.NetworkName = d.host.NetworkName()
-	mb.SigningMarshaler = d.host
 	msg, err := mb.Build(context.Background(), d.host, partialMessage.Sender)
 	d.require.NoError(err)
 	d.require.NotNil(msg)

emulator/instance.go

@@ -146,7 +146,7 @@ func (i *Instance) NewJustification(round uint64, phase gpbft.Phase, vote *gpbft
 }
 
 func (i *Instance) NewJustificationWithPayload(payload gpbft.Payload, from ...gpbft.ActorID) *gpbft.Justification {
-	msg := i.signing.MarshalPayloadForSigning(networkName, &payload)
+	msg := payload.MarshalForSigning(networkName)
 	qr := gpbft.QuorumResult{
 		Signers:    make([]int, len(from)),
 		Signatures: make([][]byte, len(from)),

emulator/signing.go

@@ -19,7 +19,6 @@ var (
 type Signing interface {
 	gpbft.Verifier
 	gpbft.Signer
-	gpbft.SigningMarshaler
 }
 
 // AdhocSigning marshals, signs and verifies messages on behalf of any given

gpbft/api.go

@@ -123,15 +123,6 @@ type Signer interface {
 	Sign(ctx context.Context, sender PubKey, msg []byte) ([]byte, error)
 }
 
-type SigningMarshaler interface {
-	// MarshalPayloadForSigning marshals the given payload into the bytes that should be signed.
-	// This should usually call `Payload.MarshalForSigning(NetworkName)` except when testing as
-	// that method is slow (computes a merkle tree that's necessary for testing).
-	// Implementations must be safe for concurrent use.
-	// WARNING: the partial message code depends on the structure of signed payload
-	MarshalPayloadForSigning(NetworkName, *Payload) []byte
-}
-
 type Aggregate interface {
 	// Aggregates signatures from a participants.
 	//
@@ -155,11 +146,6 @@ type Verifier interface {
 	Aggregate(pubKeys []PubKey) (Aggregate, error)
 }
 
-type Signatures interface {
-	SigningMarshaler
-	Verifier
-}
-
 type DecisionReceiver interface {
 	// Receives a finality decision from the instance, with signatures from a strong quorum
 	// of participants justifying it.
@@ -182,6 +168,6 @@ type Host interface {
 	CommitteeProvider
 	Network
 	Clock
-	Signatures
+	Verifier
 	DecisionReceiver
 }

gpbft/gpbft.go

@@ -893,11 +893,10 @@ func (i *instance) broadcast(round uint64, phase Phase, value *ECChain, createTi
 	}
 
 	mb := &MessageBuilder{
-		NetworkName:      i.participant.host.NetworkName(),
-		PowerTable:       i.powerTable,
-		SigningMarshaler: i.participant.host,
-		Payload:          p,
-		Justification:    justification,
+		NetworkName:   i.participant.host.NetworkName(),
+		PowerTable:    i.powerTable,
+		Payload:       p,
+		Justification: justification,
 	}
 	if createTicket {
 		mb.BeaconForTicket = i.beacon

gpbft/message_builder.go

@@ -10,12 +10,11 @@ import (
 var ErrNoPower = errors.New("no power")
 
 type MessageBuilder struct {
-	NetworkName      NetworkName
-	PowerTable       powerTableAccessor
-	Payload          Payload
-	BeaconForTicket  []byte
-	Justification    *Justification
-	SigningMarshaler SigningMarshaler
+	NetworkName     NetworkName
+	PowerTable      powerTableAccessor
+	Payload         Payload
+	BeaconForTicket []byte
+	Justification   *Justification
 }
 
 type powerTableAccessor interface {
@@ -64,7 +63,7 @@ func (mb *MessageBuilder) PrepareSigningInputs(id ActorID) (*SignatureBuilder, e
 		PubKey: pubKey,
 	}
 
-	sb.PayloadToSign = mb.SigningMarshaler.MarshalPayloadForSigning(mb.NetworkName, &mb.Payload)
+	sb.PayloadToSign = mb.Payload.MarshalForSigning(mb.NetworkName)
 	if mb.BeaconForTicket != nil {
 		sb.VRFToSign = vrfSerializeSigInput(mb.BeaconForTicket, mb.Payload.Instance, mb.Payload.Round, mb.NetworkName)
 	}

gpbft/message_builder_test.go

@@ -8,14 +8,6 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-type testSigningMarshaler struct{}
-
-var signingMarshaler SigningMarshaler = testSigningMarshaler{}
-
-func (testSigningMarshaler) MarshalPayloadForSigning(nn NetworkName, p *Payload) []byte {
-	return p.MarshalForSigning(nn)
-}
-
 func TestMessageBuilder(t *testing.T) {
 	pt := NewPowerTable()
 	err := pt.Add([]PowerEntry{
@@ -38,10 +30,9 @@ func TestMessageBuilder(t *testing.T) {
 	nn := NetworkName("test")
 
 	mt := &MessageBuilder{
-		NetworkName:      nn,
-		PowerTable:       pt,
-		Payload:          payload,
-		SigningMarshaler: signingMarshaler,
+		NetworkName: nn,
+		PowerTable:  pt,
+		Payload:     payload,
 	}
 
 	_, err = mt.PrepareSigningInputs(2)
@@ -88,11 +79,10 @@ func TestMessageBuilderWithVRF(t *testing.T) {
 
 	nn := NetworkName("test")
 	mt := &MessageBuilder{
-		NetworkName:      nn,
-		PowerTable:       pt,
-		Payload:          payload,
-		SigningMarshaler: signingMarshaler,
-		BeaconForTicket:  []byte{0xbe, 0xac, 0x04},
+		NetworkName:     nn,
+		PowerTable:      pt,
+		Payload:         payload,
+		BeaconForTicket: []byte{0xbe, 0xac, 0x04},
 	}
 
 	st, err := mt.PrepareSigningInputs(0)

gpbft/mock_host_test.go

@@ -24,7 +24,7 @@ type cachingValidator struct {
 	committeeLookback uint64
 	committeeProvider CommitteeProvider
 	networkName       NetworkName
-	signing           Signatures
+	verifier          Verifier
 	progress          Progress
 }
 
@@ -34,7 +34,7 @@ func newValidator(host Host, cp CommitteeProvider, progress Progress, cache *cac
 		committeeProvider: cp,
 		committeeLookback: committeeLookback,
 		networkName:       host.NetworkName(),
-		signing:           host,
+		verifier:          host,
 		progress:          progress,
 	}
 }
@@ -128,7 +128,7 @@ func (v *cachingValidator) ValidateMessage(msg *GMessage) (valid ValidatedMessag
 		if msg.Vote.Value.IsZero() {
 			return nil, fmt.Errorf("unexpected zero value for converge phase: %w", ErrValidationInvalid)
 		}
-		if !VerifyTicket(v.networkName, comt.Beacon, msg.Vote.Instance, msg.Vote.Round, senderPubKey, v.signing, msg.Ticket) {
+		if !VerifyTicket(v.networkName, comt.Beacon, msg.Vote.Instance, msg.Vote.Round, senderPubKey, v.verifier, msg.Ticket) {
 			return nil, fmt.Errorf("failed to verify ticket from %v: %w", msg.Sender, ErrValidationInvalid)
 		}
 	case DECIDE_PHASE:
@@ -145,8 +145,8 @@ func (v *cachingValidator) ValidateMessage(msg *GMessage) (valid ValidatedMessag
 	}
 
 	// Check vote signature.
-	sigPayload := v.signing.MarshalPayloadForSigning(v.networkName, &msg.Vote)
-	if err := v.signing.Verify(senderPubKey, sigPayload, msg.Signature); err != nil {
+	sigPayload := msg.Vote.MarshalForSigning(v.networkName)
+	if err := v.verifier.Verify(senderPubKey, sigPayload, msg.Signature); err != nil {
 		return nil, fmt.Errorf("invalid signature on %v, %v: %w", msg, err, ErrValidationInvalid)
 	}
 
@@ -272,7 +272,7 @@ func (v *cachingValidator) validateJustification(msg *GMessage, comt *Committee)
 		return fmt.Errorf("message %v has justification with insufficient power: %v", msg, justificationPower)
 	}
 
-	payload := v.signing.MarshalPayloadForSigning(v.networkName, &msg.Justification.Vote)
+	payload := msg.Justification.Vote.MarshalForSigning(v.networkName)
 	if err := comt.AggregateVerifier.VerifyAggregate(signers, payload, msg.Justification.Signature); err != nil {
 		return fmt.Errorf("verification of the aggregate failed: %+v: %w", msg.Justification, err)
 	}

gpbft/validator.go

@@ -42,7 +42,7 @@ type cachingPartialValidator struct {
 	committeeLookback uint64
 	committeeProvider gpbft.CommitteeProvider
 	networkName       gpbft.NetworkName
-	signing           gpbft.Signatures
+	signing           gpbft.Verifier
 	progress          gpbft.Progress
 }
 

partial_validator.go

@@ -87,7 +87,7 @@ func (i *ImmediateDecide) StartInstanceAt(instance uint64, _when time.Time) erro
 	if i.supplementalData != nil {
 		justificationPayload.SupplementalData = *i.supplementalData
 	}
-	sigPayload := i.host.MarshalPayloadForSigning(i.host.NetworkName(), &justificationPayload)
+	sigPayload := justificationPayload.MarshalForSigning(i.host.NetworkName())
 	signers := bitfield.New()
 
 	signers.Set(uint64(committee.PowerTable.Lookup[i.id]))
@@ -129,9 +129,8 @@ func (i *ImmediateDecide) StartInstanceAt(instance uint64, _when time.Time) erro
 
 	// Immediately send a DECIDE message
 	mb := &gpbft.MessageBuilder{
-		NetworkName:      i.host.NetworkName(),
-		PowerTable:       committee.PowerTable,
-		SigningMarshaler: i.host,
+		NetworkName: i.host.NetworkName(),
+		PowerTable:  committee.PowerTable,
 		Payload: gpbft.Payload{
 			Instance:         instance,
 			Round:            0,

sim/adversary/decide.go

@@ -95,11 +95,10 @@ func (r *Repeat) ReceiveMessage(vmsg gpbft.ValidatedMessage) error {
 		Value:            msg.Vote.Value,
 	}
 	mt := &gpbft.MessageBuilder{
-		NetworkName:      r.host.NetworkName(),
-		PowerTable:       committee.PowerTable,
-		Payload:          p,
-		Justification:    msg.Justification,
-		SigningMarshaler: r.host,
+		NetworkName:   r.host.NetworkName(),
+		PowerTable:    committee.PowerTable,
+		Payload:       p,
+		Justification: msg.Justification,
 	}
 	if len(msg.Ticket) > 0 {
 		mt.BeaconForTicket = committee.Beacon

sim/adversary/repeat.go

@@ -79,10 +79,9 @@ func (s *Spam) spamAtInstance(instance uint64) {
 			Phase:            gpbft.COMMIT_PHASE,
 		}
 		mt := &gpbft.MessageBuilder{
-			NetworkName:      s.host.NetworkName(),
-			PowerTable:       committee.PowerTable,
-			Payload:          p,
-			SigningMarshaler: s.host,
+			NetworkName: s.host.NetworkName(),
+			PowerTable:  committee.PowerTable,
+			Payload:     p,
 		}
 		if err := s.host.RequestBroadcast(mt); err != nil {
 			panic(err)

sim/adversary/spam.go

@@ -107,7 +107,7 @@ func (w *WithholdCommit) StartInstanceAt(instance uint64, _when time.Time) error
 
 	signatures := make([][]byte, 0)
 	mask := make([]int, 0)
-	prepareMarshalled := w.host.MarshalPayloadForSigning(w.host.NetworkName(), &preparePayload)
+	prepareMarshalled := preparePayload.MarshalForSigning(w.host.NetworkName())
 	for _, signerIndex := range signers {
 		entry := committee.PowerTable.Entries[signerIndex]
 		signatures = append(signatures, w.sign(entry.PubKey, prepareMarshalled))
@@ -181,11 +181,10 @@ func (w *WithholdCommit) sign(pubkey gpbft.PubKey, msg []byte) []byte {
 func (w *WithholdCommit) synchronousBroadcastRequester(powertable *gpbft.PowerTable) func(gpbft.Payload, *gpbft.Justification) {
 	return func(payload gpbft.Payload, justification *gpbft.Justification) {
 		mb := &gpbft.MessageBuilder{
-			NetworkName:      w.host.NetworkName(),
-			PowerTable:       powertable,
-			Payload:          payload,
-			Justification:    justification,
-			SigningMarshaler: w.host,
+			NetworkName:   w.host.NetworkName(),
+			PowerTable:    powertable,
+			Payload:       payload,
+			Justification: justification,
 		}
 		if err := w.host.RequestSynchronousBroadcast(mb); err != nil {
 			panic(err)