Merge pull request #2631 from oasislabs/kostko/fix/epochtime-overflow

go/consensus/tendermint/apps/staking: Fix epochtime overflow

.changelog/2627.bugfix.md

@@ -0,0 +1 @@
+go/consensus/tendermint/apps/staking: Fix epochtime overflow.

go/consensus/tendermint/apps/registry/registry.go

@@ -3,6 +3,7 @@ package registry
 
 import (
 	"fmt"
+	"math"
 
 	"github.com/tendermint/tendermint/abci/types"
 
@@ -170,6 +171,10 @@ func (app *registryApplication) onRegistryEpochChanged(ctx *abci.Context, regist
 		}
 
 		// If node has been expired for the debonding interval, finally remove it.
+		if math.MaxUint64-node.Expiration < uint64(debondingInterval) {
+			// Overflow, the node will never be removed.
+			continue
+		}
 		if epochtime.EpochTime(node.Expiration)+debondingInterval < registryEpoch {
 			ctx.Logger().Debug("removing expired node",
 				"node_id", node.ID,

go/consensus/tendermint/apps/staking/slashing.go

@@ -3,6 +3,7 @@ package staking
 import (
 	"context"
 	"encoding/hex"
+	"math"
 	"time"
 
 	tmcrypto "github.com/tendermint/tendermint/crypto"
@@ -11,10 +12,11 @@ import (
 	registryState "github.com/oasislabs/oasis-core/go/consensus/tendermint/apps/registry/state"
 	stakingState "github.com/oasislabs/oasis-core/go/consensus/tendermint/apps/staking/state"
 	epochtime "github.com/oasislabs/oasis-core/go/epochtime/api"
+	registry "github.com/oasislabs/oasis-core/go/registry/api"
 	staking "github.com/oasislabs/oasis-core/go/staking/api"
 )
 
-func (app *stakingApplication) onEvidenceDoubleSign(
+func onEvidenceDoubleSign(
 	ctx *abci.Context,
 	addr tmcrypto.Address,
 	height int64,
@@ -70,12 +72,17 @@ func (app *stakingApplication) onEvidenceDoubleSign(
 	// validator from being scheduled in the next epoch.
 	if penalty.FreezeInterval > 0 {
 		var epoch epochtime.EpochTime
-		epoch, err = app.state.GetEpoch(context.Background(), ctx.BlockHeight()+1)
+		epoch, err = ctx.AppState().GetEpoch(context.Background(), ctx.BlockHeight()+1)
 		if err != nil {
 			return err
 		}
 
-		nodeStatus.FreezeEndTime = epoch + penalty.FreezeInterval
+		// Check for overflow.
+		if math.MaxUint64-penalty.FreezeInterval < epoch {
+			nodeStatus.FreezeEndTime = registry.FreezeForever
+		} else {
+			nodeStatus.FreezeEndTime = epoch + penalty.FreezeInterval
+		}
 	}
 
 	// Slash validator.

go/consensus/tendermint/apps/staking/slashing_test.go

@@ -0,0 +1,120 @@
+package staking
+
+import (
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/require"
+
+	"github.com/oasislabs/oasis-core/go/common/crypto/signature"
+	memorySigner "github.com/oasislabs/oasis-core/go/common/crypto/signature/signers/memory"
+	"github.com/oasislabs/oasis-core/go/common/entity"
+	"github.com/oasislabs/oasis-core/go/common/node"
+	"github.com/oasislabs/oasis-core/go/common/quantity"
+	"github.com/oasislabs/oasis-core/go/consensus/tendermint/abci"
+	registryState "github.com/oasislabs/oasis-core/go/consensus/tendermint/apps/registry/state"
+	stakingState "github.com/oasislabs/oasis-core/go/consensus/tendermint/apps/staking/state"
+	tmcrypto "github.com/oasislabs/oasis-core/go/consensus/tendermint/crypto"
+	registry "github.com/oasislabs/oasis-core/go/registry/api"
+	staking "github.com/oasislabs/oasis-core/go/staking/api"
+)
+
+func TestOnEvidenceDoubleSign(t *testing.T) {
+	require := require.New(t)
+
+	now := time.Unix(1580461674, 0)
+	appState := abci.NewMockApplicationState(abci.MockApplicationStateConfig{
+		// Use a non-zero current epoch so we test freeze overflow.
+		CurrentEpoch: 42,
+	})
+	ctx := appState.NewContext(abci.ContextBeginBlock, now)
+	defer ctx.Close()
+
+	consensusSigner := memorySigner.NewTestSigner("consensus test signer")
+	consensusID := consensusSigner.Public()
+	validatorAddress := tmcrypto.PublicKeyToTendermint(&consensusID).Address()
+
+	regState := registryState.NewMutableState(ctx.State())
+	stakeState := stakingState.NewMutableState(ctx.State())
+
+	// Validator address is not known as there are no nodes.
+	err := onEvidenceDoubleSign(ctx, validatorAddress, 1, now, 1)
+	require.NoError(err, "should not fail when validator address is not known")
+
+	// Add entity.
+	ent, entitySigner, _ := entity.TestEntity()
+	sigEntity, err := entity.SignEntity(entitySigner, registry.RegisterEntitySignatureContext, ent)
+	require.NoError(err, "SignEntity")
+	regState.SetEntity(ent, sigEntity)
+	// Add node.
+	nodeSigner := memorySigner.NewTestSigner("node test signer")
+	nod := &node.Node{
+		ID:       nodeSigner.Public(),
+		EntityID: ent.ID,
+		Consensus: node.ConsensusInfo{
+			ID: consensusID,
+		},
+	}
+	sigNode, err := node.MultiSignNode([]signature.Signer{nodeSigner}, registry.RegisterNodeSignatureContext, nod)
+	require.NoError(err, "MultiSignNode")
+	err = regState.SetNode(nod, sigNode)
+	require.NoError(err, "SetNode")
+
+	// Should not fail if node status is not available.
+	err = onEvidenceDoubleSign(ctx, validatorAddress, 1, now, 1)
+	require.NoError(err, "should not fail when node status is not available")
+
+	// Add node status.
+	err = regState.SetNodeStatus(nod.ID, &registry.NodeStatus{})
+	require.NoError(err, "SetNodeStatus")
+
+	// Should fail if unable to get the slashing procedure.
+	err = onEvidenceDoubleSign(ctx, validatorAddress, 1, now, 1)
+	require.Error(err, "should fail when unable to get the slashing procedure")
+
+	// Add slashing procedure.
+	var slashAmount quantity.Quantity
+	_ = slashAmount.FromUint64(100)
+	stakeState.SetConsensusParameters(&staking.ConsensusParameters{
+		Slashing: map[staking.SlashReason]staking.Slash{
+			staking.SlashDoubleSigning: staking.Slash{
+				Amount:         slashAmount,
+				FreezeInterval: registry.FreezeForever,
+			},
+		},
+	})
+
+	// Should fail as the validator has no stake (which is an invariant violation as a validator
+	// needs to have some stake).
+	err = onEvidenceDoubleSign(ctx, validatorAddress, 1, now, 1)
+	require.Error(err, "should fail when validator has no stake")
+
+	// Get the validator some stake.
+	var balance quantity.Quantity
+	_ = balance.FromUint64(200)
+	var totalShares quantity.Quantity
+	_ = totalShares.FromUint64(200)
+	stakeState.SetAccount(ent.ID, &staking.Account{
+		Escrow: staking.EscrowAccount{
+			Active: staking.SharePool{
+				Balance:     balance,
+				TotalShares: totalShares,
+			},
+		},
+	})
+
+	// Should slash.
+	err = onEvidenceDoubleSign(ctx, validatorAddress, 1, now, 1)
+	require.NoError(err, "slashing should succeed")
+
+	// Entity stake should be slashed.
+	acct := stakeState.Account(ent.ID)
+	_ = balance.Sub(&slashAmount)
+	require.EqualValues(balance, acct.Escrow.Active.Balance, "entity stake should be slashed")
+
+	// Node should be frozen.
+	status, err := regState.NodeStatus(nod.ID)
+	require.NoError(err, "NodeStatus")
+	require.True(status.IsFrozen(), "node should be frozen after slashing")
+	require.EqualValues(registry.FreezeForever, status.FreezeEndTime, "node should be frozen forever")
+}

go/consensus/tendermint/apps/staking/staking.go

@@ -70,7 +70,7 @@ func (app *stakingApplication) BeginBlock(ctx *abci.Context, request types.Reque
 	for _, evidence := range request.ByzantineValidators {
 		switch evidence.Type {
 		case tmtypes.ABCIEvidenceTypeDuplicateVote:
-			if err := app.onEvidenceDoubleSign(ctx, evidence.Validator.Address, evidence.Height, evidence.Time, evidence.Validator.Power); err != nil {
+			if err := onEvidenceDoubleSign(ctx, evidence.Validator.Address, evidence.Height, evidence.Time, evidence.Validator.Power); err != nil {
 				return err
 			}
 		default: