Use big.Int encoded as string for vote weights (#506)

* Use big.Int encoded as string for vote weights
* Check validity of externally received memberships
* Use large node weights in Trantor tests

Signed-off-by: Matej Pavlovic <[email protected]>

cmd/mircat/debug.go

@@ -44,7 +44,7 @@ func debug(args *arguments) error {
 			Id:     nID,
 			Addr:   libp2p.NewDummyHostAddr(0, 0).String(),
 			Key:    nil,
-			Weight: 1,
+			Weight: "1",
 		}
 	}
 

pkg/availability/multisigcollector/internal/parts/certverification/certverification.go

@@ -133,7 +133,7 @@ func verifyCertificateStructure(params *common.ModuleParams, cert *apbtypes.Cert
 
 		// Check that a quorum of nodes signed the certificate.
 		if !membutil.HaveWeakQuorum(params.Membership, mscCert.Signers) {
-			return nil, es.Errorf("insufficient weight of signatures: %d, need %d (signers: %v)",
+			return nil, es.Errorf("insufficient weight of signatures: %v, need %v (signers: %v)",
 				membutil.WeightOf(params.Membership, mscCert.Signers),
 				membutil.WeakQuorum(params.Membership),
 				mscCert.Signers,

pkg/checkpoint/checkpoint.go

@@ -227,10 +227,6 @@ func (sc *StableCheckpoint) verifyMembershipConsistency(membership *trantorpbtyp
 		for nodeID, node := range chkpMembership.Nodes {
 			if _, ok := membershipConsistency[nodeID]; !ok {
 				membershipConsistency[nodeID] = node
-				// check that internal nodeID is consistent with key nodeID
-				if nodeID != node.Id {
-					return es.Errorf("inconsistent membership parameters: nodeID %v does not match internal nodeID %v", nodeID, node.Id)
-				}
 			} else {
 				// check that all parameters are consistent
 				if !reflect.DeepEqual(membershipConsistency[nodeID], node) {
@@ -302,10 +298,23 @@ func (sc *StableCheckpoint) SyntacticCheck(
 			len(sc.Memberships()))
 	}
 
+	// Check the contained memberships for validity.
+	for _, membership := range sc.Memberships() {
+		if err := membutil.Valid(membership); err != nil {
+			return es.Errorf("invalid membership: %w", err)
+		}
+	}
+
 	if sc.PreviousMembership() == nil {
 		return es.Errorf("previous membership is nil")
 	}
 
+	if sc.Epoch() > 0 {
+		if err := membutil.Valid(sc.PreviousMembership()); err != nil {
+			return es.Errorf("invalid previous membership: %w", err)
+		}
+	}
+
 	if sc.Certificate() == nil {
 		return es.Errorf("certificate is nil")
 	}

pkg/checkpoint/serializing_fuzz_test.go

@@ -34,7 +34,7 @@ func FuzzSnapshotForHash(f *testing.F) {
 				Id:     id,
 				Addr:   addr,
 				Key:    nil,
-				Weight: 1,
+				Weight: "1",
 			}
 		}
 

pkg/deploytest/helpers.go

@@ -19,7 +19,7 @@ func NewNodeIDsWeights(nNodes int, weightFunction func(t.NodeID) types.VoteWeigh
 
 func NewNodeIDsDefaultWeights(nNodes int) map[t.NodeID]types.VoteWeight {
 	return NewNodeIDsWeights(nNodes, func(t.NodeID) types.VoteWeight {
-		return 1
+		return "100000000000000000000" // This does not fit in 64 bits
 	})
 }
 

pkg/iss/config/config.go

@@ -13,6 +13,7 @@ import (
 
 	lsp "github.com/filecoin-project/mir/pkg/iss/leaderselectionpolicy"
 	trantorpbtypes "github.com/filecoin-project/mir/pkg/pb/trantorpb/types"
+	"github.com/filecoin-project/mir/pkg/util/membutil"
 )
 
 // The ModuleParams type defines all the ISS configuration parameters.
@@ -94,9 +95,9 @@ type ModuleParams struct {
 // CheckParams checks whether the given configuration satisfies all necessary constraints.
 func CheckParams(c *ModuleParams) error {
 
-	// The membership must not be empty.
-	if len(c.InitialMembership.Nodes) == 0 {
-		return es.Errorf("empty membership")
+	// The membership must be valid.
+	if err := membutil.Valid(c.InitialMembership); err != nil {
+		return es.Errorf("invalid initial membership: %w", err)
 	}
 
 	// Check that ConfigOffset is at least 1

pkg/iss/leaderselectionpolicy/leaderselectionpolicy_test.go

@@ -1,7 +1,6 @@
 package leaderselectionpolicy
 
 import (
-	"sort"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -42,7 +41,7 @@ func TestSimpleLeaderPolicy(t *testing.T) {
 
 func TestBlackListLeaderPolicy(t *testing.T) {
 	// Create a BlacklistLeaderPolicy with 3 nodes and a minimum of 2 leaders
-	nodes := testMembership([]types.NodeID{"node1", "node2", "node3"})
+	nodes := testMembership([]types.NodeID{"node1", "node2", "node3", "node4"})
 	policy := NewBlackListLeaderPolicy(nodes)
 
 	// Check that all nodes are returned as leaders
@@ -52,27 +51,27 @@ func TestBlackListLeaderPolicy(t *testing.T) {
 
 	// Suspect a node and check that it is not returned as a leader
 	policy.Suspect(1, "node1")
-	expected = []types.NodeID{"node2", "node3"}
+	expected = []types.NodeID{"node2", "node3", "node4"}
 	actual = policy.Leaders()
-	sort.Slice(expected, func(i, j int) bool {
-		return string(expected[i]) < string(expected[j])
-	})
-	sort.Slice(actual, func(i, j int) bool {
-		return string(actual[i]) < string(actual[j])
-	})
 	require.Equal(t, expected, actual)
 
-	// Suspect another node and check that it is not returned as a leader
-	policy.Suspect(2, "node2")
-	expected = []types.NodeID{"node3", "node1"}
+	// Suspect another node and check that the previously suspected node is again returned as a leader.
+	policy.Suspect(2, "node4")
+	expected = []types.NodeID{"node2", "node3", "node1"}
 	actual = policy.Leaders()
 	require.Equal(t, expected, actual)
 
-	// Reconfigure the policy with a new set of nodes and check that it is returned as the leader set
-	newNodes := testMembership([]types.NodeID{"node4", "node5", "node6"})
-	policy, ok := policy.Reconfigure(newNodes).(*BlacklistLeaderPolicy)
+	// Reconfigure the policy with a new set of nodes and check that it is returned as the leader set.
+	policy, ok := policy.Reconfigure(testMembership([]types.NodeID{"node1", "node5"})).(*BlacklistLeaderPolicy)
 	require.Equal(t, ok, true)
-	expected = maputil.GetSortedKeys(newNodes.Nodes)
+	expected = []types.NodeID{"node5", "node1"}
+	actual = policy.Leaders()
+	require.Equal(t, expected, actual)
+
+	// Add one more node and check that the suspected node is removed again from the leader set.
+	policy, ok = policy.Reconfigure(testMembership([]types.NodeID{"node1", "node5", "node6"})).(*BlacklistLeaderPolicy)
+	require.Equal(t, ok, true)
+	expected = []types.NodeID{"node5", "node6"}
 	actual = policy.Leaders()
 	require.Equal(t, expected, actual)
 }
@@ -142,7 +141,7 @@ func testMembership(nodeIDs []types.NodeID) *trantorpbtypes.Membership {
 			Id:     nodeID,
 			Addr:   "",
 			Key:    nil,
-			Weight: 1,
+			Weight: "1",
 		}
 	}
 	return &m

pkg/membership/bootstrap.go

@@ -74,7 +74,7 @@ func FromFile(f *os.File) (*trantorpbtypes.Membership, error) {
 		Identities []struct {
 			ID     string `json:"addr"`
 			Addr   string `json:"net_addr"`
-			Weight uint64 `json:"weight,string"`
+			Weight string `json:"weight,string"`
 		} `json:"validators"`
 	}{}
 
@@ -140,7 +140,7 @@ func DummyMultiAddrs(membershipIn *trantorpbtypes.Membership) (*trantorpbtypes.M
 			identity.Id,
 			libp2ptools.NewDummyMultiaddr(numericID, newAddr).String(),
 			nil,
-			1,
+			"1",
 		}
 	}
 

pkg/net/libp2p/transport_test.go

@@ -59,7 +59,7 @@ func newMockLibp2pCommunication(
 			id,
 			libp2putil.NewDummyMultiaddr(i, hostAddr).String(),
 			nil,
-			1,
+			"1",
 		}
 		lt.transports[id] = NewTransport(params, id, lt.hosts[id], logger)
 	}
@@ -870,8 +870,8 @@ func TestOpeningConnectionAfterFail(t *testing.T) {
 	b := NewTransport(DefaultParams(), nodeB, hostB, logger)
 
 	membershipA := &trantorpbtypes.Membership{make(map[types.NodeID]*trantorpbtypes.NodeIdentity, 2)} // nolint:govet
-	membershipA.Nodes[nodeA] = &trantorpbtypes.NodeIdentity{nodeA, addrA.String(), nil, 1}            // nolint:govet
-	membershipA.Nodes[nodeB] = &trantorpbtypes.NodeIdentity{nodeB, addrB.String(), nil, 1}            // nolint:govet
+	membershipA.Nodes[nodeA] = &trantorpbtypes.NodeIdentity{nodeA, addrA.String(), nil, "1"}          // nolint:govet
+	membershipA.Nodes[nodeB] = &trantorpbtypes.NodeIdentity{nodeB, addrB.String(), nil, "1"}          // nolint:govet
 
 	require.Equal(t, nodeA, a.ownID)
 	require.Equal(t, nodeB, b.ownID)

pkg/trantor/appmodule/appmodule.go

@@ -15,6 +15,7 @@ import (
 	trantorpbtypes "github.com/filecoin-project/mir/pkg/pb/trantorpb/types"
 	"github.com/filecoin-project/mir/pkg/trantor/types"
 	t "github.com/filecoin-project/mir/pkg/types"
+	"github.com/filecoin-project/mir/pkg/util/membutil"
 )
 
 // AppModule is the module within the SMR system that handles the application logic.
@@ -71,6 +72,11 @@ func NewAppModule(appLogic AppLogic, transport net.Transport, moduleID t.ModuleI
 		if err != nil {
 			return es.Errorf("error handling NewEpoch event: %w", err)
 		}
+
+		if err = membutil.Valid(membership); err != nil {
+			return es.Errorf("app logic provided invalid membership: %w", err)
+		}
+
 		appModule.transport.Connect(membership)
 		isspbdsl.NewConfig(m, protocolModule, epochNr, membership)
 		return nil

pkg/trantor/testing/smr_test.go

@@ -3,7 +3,6 @@ package testing
 import (
 	"context"
 	"fmt"
-	"math"
 	"math/rand"
 	"os"
 	"path/filepath"
@@ -188,8 +187,8 @@ func testIntegrationWithISS(tt *testing.T) {
 		13: {"Submit 100 fake transactions with 4 nodes in simulation, two of them crashed (not messages sent, yes received) and holding the supermajority of stake",
 			&TestConfig{
 				NodeIDsWeight: deploytest.NewNodeIDsWeights(4, func(id t.NodeID) types.VoteWeight {
-					idfloat, _ := strconv.ParseFloat(id.Pb(), 64)
-					return types.VoteWeight(math.Pow(2, idfloat)) // ensures last 2 nodes weight is greater than twice the sum of the others'
+					numericID, _ := strconv.ParseInt(id.Pb(), 10, 64)
+					return types.VoteWeight(fmt.Sprintf("%d0000000000000000000", pow2(int(numericID)))) // ensures last 2 nodes weight is greater than twice the sum of the others'
 				}),
 				NumClients:      0,
 				Transport:       "libp2p",
@@ -208,8 +207,8 @@ func testIntegrationWithISS(tt *testing.T) {
 		14: {"Submit 100 fake transactions with 4 nodes in simulation, two of them crashed (no messages sent, yes received) but holding the minority of stake",
 			&TestConfig{
 				NodeIDsWeight: deploytest.NewNodeIDsWeights(4, func(id t.NodeID) types.VoteWeight {
-					idfloat, _ := strconv.ParseFloat(id.Pb(), 64)
-					return types.VoteWeight(math.Pow(2, 4-idfloat)) // ensures first 2 nodes weight is greater than twice the sum of the others'
+					numericID, _ := strconv.ParseInt(id.Pb(), 10, 64)
+					return types.VoteWeight(fmt.Sprintf("%d0000000000000000000", pow2(int(4-numericID)))) // ensures first 2 nodes weight is greater than twice the sum of the others'
 				}),
 				NumClients:      0,
 				Transport:       "libp2p",
@@ -518,3 +517,13 @@ func newDeployment(conf *TestConfig) (*deploytest.Deployment, error) {
 
 	return deploytest.NewDeployment(deployConf)
 }
+
+func pow2(exp int) uint64 {
+	y := uint64(1)
+
+	for i := 0; i < exp; i++ {
+		y *= 2
+	}
+
+	return y
+}

pkg/trantor/types/types.go

@@ -1,6 +1,7 @@
 package types
 
 import (
+	"math/big"
 	"strconv"
 
 	"github.com/filecoin-project/mir/pkg/serializing"
@@ -9,10 +10,41 @@ import (
 // ================================================================================
 
 // VoteWeight represents the weight of a node's vote when gathering quorums.
-type VoteWeight uint64
+// The underlying type is a string containing a decimal integer representation of the weight.
+// This is required to support large integers that would not fit in a native data type like uint64.
+// For example, this can occur if Trantor is used as a PoS system with cryptocurrency units as weights.
+// We do not store the weights directly as big.Int, since that would make it harder to use them in protocol buffers.
+// Instead, when performing mathematical operations on weights, we convert them to the big.Int type.
+type VoteWeight string
 
 func (vw VoteWeight) Bytes() []byte {
-	return serializing.Uint64ToBytes(uint64(vw))
+	return []byte(vw)
+}
+
+func (vw VoteWeight) Pb() string {
+	return string(vw)
+}
+
+func (vw VoteWeight) String() string {
+	return string(vw)
+}
+
+func (vw VoteWeight) IsValid() bool {
+	if _, ok := new(big.Int).SetString(vw.String(), 10); !ok {
+		return false
+	}
+	return true
+}
+
+// BigInt converts a VoteWeight (normally represented as a string) to a big.Int.
+// BigInt panics if the underlying string is not a valid decimal representation of an integer.
+// Thus, BigInt must not be called on received input without having validated VoteWeight by calling the IsValid method.
+func (vw VoteWeight) BigInt() *big.Int {
+	var bi big.Int
+	if _, ok := bi.SetString(vw.String(), 10); !ok {
+		panic("invalid vote weight representation")
+	}
+	return &bi
 }
 
 // ================================================================================