From 956168c0474d2406bd4a7b5e110df7dd13a27403 Mon Sep 17 00:00:00 2001 From: Jernej Kos Date: Fri, 31 Jan 2020 13:04:19 +0100 Subject: [PATCH 1/2] go/consensus/tendermint/apps/staking: Fix epochtime overflow --- .changelog/2627.bugfix.md | 1 + .../tendermint/apps/staking/slashing.go | 13 +- .../tendermint/apps/staking/slashing_test.go | 120 ++++++++++++++++++ .../tendermint/apps/staking/staking.go | 2 +- 4 files changed, 132 insertions(+), 4 deletions(-) create mode 100644 .changelog/2627.bugfix.md create mode 100644 go/consensus/tendermint/apps/staking/slashing_test.go diff --git a/.changelog/2627.bugfix.md b/.changelog/2627.bugfix.md new file mode 100644 index 00000000000..0ad3123d295 --- /dev/null +++ b/.changelog/2627.bugfix.md @@ -0,0 +1 @@ +go/consensus/tendermint/apps/staking: Fix epochtime overflow. diff --git a/go/consensus/tendermint/apps/staking/slashing.go b/go/consensus/tendermint/apps/staking/slashing.go index 6355035f510..fe09d726609 100644 --- a/go/consensus/tendermint/apps/staking/slashing.go +++ b/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. diff --git a/go/consensus/tendermint/apps/staking/slashing_test.go b/go/consensus/tendermint/apps/staking/slashing_test.go new file mode 100644 index 00000000000..97c44d08349 --- /dev/null +++ b/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, ®istry.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") +} diff --git a/go/consensus/tendermint/apps/staking/staking.go b/go/consensus/tendermint/apps/staking/staking.go index b8df70d191a..71a533bd26d 100644 --- a/go/consensus/tendermint/apps/staking/staking.go +++ b/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: From 8b7ef7a2623fd2887ba6eb40b440b2b0c85f7364 Mon Sep 17 00:00:00 2001 From: Jernej Kos Date: Fri, 31 Jan 2020 13:47:19 +0100 Subject: [PATCH 2/2] go/consensus/tendermint/apps/registry: Fix unlikely epochtime overflow --- go/consensus/tendermint/apps/registry/registry.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/go/consensus/tendermint/apps/registry/registry.go b/go/consensus/tendermint/apps/registry/registry.go index edd0c222ff5..ddea2680249 100644 --- a/go/consensus/tendermint/apps/registry/registry.go +++ b/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,