diff --git a/core/beacon_slash_validate.go b/core/beacon_slash_validate.go new file mode 100644 index 0000000000..7c71999f3a --- /dev/null +++ b/core/beacon_slash_validate.go @@ -0,0 +1,42 @@ +package core + +import ( + "errors" + "math/big" + + "github.com/ethereum/go-ethereum/common" + "github.com/harmony-one/harmony/crypto/hash" + "github.com/harmony-one/harmony/internal/params" + "github.com/harmony-one/harmony/staking/slash" +) + +var errInvalidBeaconSlashPayload = errors.New("invalid beacon slash payload in header") + +// checkBeaconSlashEvidenceUniqueness enforces canonical uniqueness for decoded +// beacon slash records when the chain schedule enables the rule. Caller runs this +// from StateProcessor.Process before the processor result cache and before +// transaction execution so all execution paths apply the same checks. +func checkBeaconSlashEvidenceUniqueness(cfg *params.ChainConfig, epoch *big.Int, records slash.Records) error { + if cfg == nil || !cfg.IsRejectDuplicateSlashEvidence(epoch) || len(records) < 2 { + return nil + } + seen := make(map[common.Hash]struct{}, len(records)*2) + for i := range records { + ev := &records[i].Evidence + h := hash.FromRLPNew256(ev) + if _, ok := seen[h]; ok { + return errInvalidBeaconSlashPayload + } + swapEv := *ev + tmp := swapEv.ConflictingVotes.FirstVote + swapEv.ConflictingVotes.FirstVote = swapEv.ConflictingVotes.SecondVote + swapEv.ConflictingVotes.SecondVote = tmp + sh := hash.FromRLPNew256(&swapEv) + if _, ok := seen[sh]; ok { + return errInvalidBeaconSlashPayload + } + seen[h] = struct{}{} + seen[sh] = struct{}{} + } + return nil +} diff --git a/core/beacon_slash_validate_test.go b/core/beacon_slash_validate_test.go new file mode 100644 index 0000000000..62eb15f738 --- /dev/null +++ b/core/beacon_slash_validate_test.go @@ -0,0 +1,97 @@ +package core + +import ( + "math/big" + "testing" + + "github.com/ethereum/go-ethereum/common" + "github.com/harmony-one/harmony/crypto/hash" + "github.com/harmony-one/harmony/internal/params" + "github.com/harmony-one/harmony/shard" + "github.com/harmony-one/harmony/staking/slash" + "github.com/stretchr/testify/require" +) + +// reporterVariantSlashRecords returns two slash records that share identical +// signed evidence but use different reporter addresses (bug07 reporter-variant clone). +func reporterVariantSlashRecords(t *testing.T) (slash.Records, common.Hash) { + t.Helper() + + evidence := slash.Evidence{ + Moment: slash.Moment{ + Epoch: big.NewInt(4), + ShardID: shard.BeaconChainShardID, + Height: 37, + ViewID: 38, + }, + ConflictingVotes: slash.ConflictingVotes{ + FirstVote: slash.Vote{ + BlockHeaderHash: common.HexToHash("0x01"), + }, + SecondVote: slash.Vote{ + BlockHeaderHash: common.HexToHash("0x02"), + }, + }, + Offender: common.HexToAddress("0x0000000000000000000000000000000000000b22"), + } + evidenceHash := hash.FromRLPNew256(evidence) + + return slash.Records{ + {Evidence: evidence, Reporter: common.HexToAddress("0x0000000000000000000000000000000000000c33")}, + {Evidence: evidence, Reporter: common.HexToAddress("0x0000000000000000000000000000000000000d44")}, + }, evidenceHash +} + +func TestCheckBeaconSlashEvidenceUniqueness_RejectsReporterVariants(t *testing.T) { + records, _ := reporterVariantSlashRecords(t) + cfg := params.TestChainConfig + + err := checkBeaconSlashEvidenceUniqueness(cfg, big.NewInt(0), records) + require.ErrorIs(t, err, errInvalidBeaconSlashPayload) +} + +func TestCheckBeaconSlashEvidenceUniqueness_SkippedBeforeFork(t *testing.T) { + records, _ := reporterVariantSlashRecords(t) + cfg := params.MainnetChainConfig + + err := checkBeaconSlashEvidenceUniqueness(cfg, big.NewInt(1_000_000), records) + require.NoError(t, err) +} + +func TestCheckBeaconSlashEvidenceUniqueness_AllowsSingleRecord(t *testing.T) { + records, _ := reporterVariantSlashRecords(t) + cfg := params.TestChainConfig + + err := checkBeaconSlashEvidenceUniqueness(cfg, big.NewInt(0), slash.Records{records[0]}) + require.NoError(t, err) +} + +func TestCheckBeaconSlashEvidenceUniqueness_AllowsDistinctEvidence(t *testing.T) { + base, _ := reporterVariantSlashRecords(t) + other := base[0] + other.Evidence.ConflictingVotes.SecondVote.BlockHeaderHash = common.HexToHash("0x03") + + records := slash.Records{base[0], other} + cfg := params.TestChainConfig + + err := checkBeaconSlashEvidenceUniqueness(cfg, big.NewInt(0), records) + require.NoError(t, err) +} + +func TestCheckBeaconSlashEvidenceUniqueness_RejectsSwappedVoteEvidence(t *testing.T) { + base, evidenceHash := reporterVariantSlashRecords(t) + swapped := base[0].Evidence + tmp := swapped.ConflictingVotes.FirstVote + swapped.ConflictingVotes.FirstVote = swapped.ConflictingVotes.SecondVote + swapped.ConflictingVotes.SecondVote = tmp + + records := slash.Records{ + base[0], + {Evidence: swapped, Reporter: common.HexToAddress("0x0000000000000000000000000000000000000e55")}, + } + cfg := params.TestChainConfig + + err := checkBeaconSlashEvidenceUniqueness(cfg, big.NewInt(0), records) + require.ErrorIs(t, err, errInvalidBeaconSlashPayload) + require.Equal(t, evidenceHash, hash.FromRLPNew256(records[0].Evidence)) +} diff --git a/core/state_processor.go b/core/state_processor.go index 0dd892b686..d5f42acf93 100644 --- a/core/state_processor.go +++ b/core/state_processor.go @@ -194,6 +194,25 @@ func (p *StateProcessor) Process( } utils.Logger().Debug().Int64("elapsed time", time.Now().Sub(startTime).Milliseconds()).Msg("Process Staking Txns") } + + // Decode header slashes and apply scheduled slash-payload rules before the + // processor result cache. When the fork flag is off, extra uniqueness checks are skipped. + var slashes slash.Records + if s := header.Slashes(); len(s) > 0 { + if err := rlp.DecodeBytes(s, &slashes); err != nil { + return nil, nil, nil, nil, 0, nil, statedb, errors.New( + "[Process] Cannot finalize block", + ) + } + } + if p.bc.ShardID() == shard.BeaconChainShardID { + if err := checkBeaconSlashEvidenceUniqueness(p.bc.Config(), block.Epoch(), slashes); err != nil { + return nil, nil, nil, nil, 0, nil, statedb, errors.WithMessage(err, + "[Process] invalid beacon slash payload", + ) + } + } + // incomingReceipts should always be processed // after transactions (to be consistent with the block proposal) for _, cx := range block.IncomingReceipts() { @@ -205,15 +224,6 @@ func (p *StateProcessor) Process( } } - slashes := slash.Records{} - if s := header.Slashes(); len(s) > 0 { - if err := rlp.DecodeBytes(s, &slashes); err != nil { - return nil, nil, nil, nil, 0, nil, statedb, errors.New( - "[Process] Cannot finalize block", - ) - } - } - if err := MayShardReduction(p.bc, statedb, header); err != nil { return nil, nil, nil, nil, 0, nil, statedb, err } diff --git a/internal/params/config.go b/internal/params/config.go index a249df5911..3a6a7d6f39 100644 --- a/internal/params/config.go +++ b/internal/params/config.go @@ -92,6 +92,7 @@ var ( EIP6780Epoch: EpochTBD, PragueEpoch: EpochTBD, EIP8024Epoch: EpochTBD, + RejectDuplicateSlashEvidenceEpoch: EpochTBD, } // TestnetChainConfig contains the chain parameters to run a node on the harmony test network. @@ -152,6 +153,7 @@ var ( EIP6780Epoch: EpochTBD, PragueEpoch: EpochTBD, EIP8024Epoch: EpochTBD, + RejectDuplicateSlashEvidenceEpoch: EpochTBD, } // PangaeaChainConfig contains the chain parameters for the Pangaea network. // All features except for CrossLink are enabled at launch. @@ -211,6 +213,7 @@ var ( EIP3860Epoch: EpochTBD, PragueEpoch: EpochTBD, EIP8024Epoch: EpochTBD, + RejectDuplicateSlashEvidenceEpoch: EpochTBD, } // PartnerChainConfig contains the chain parameters for the Partner network. @@ -272,6 +275,7 @@ var ( TimestampValidationEpoch: big.NewInt(47190), PragueEpoch: EpochTBD, EIP8024Epoch: EpochTBD, + RejectDuplicateSlashEvidenceEpoch: EpochTBD, } // StressnetChainConfig contains the chain parameters for the Stress test network. @@ -332,6 +336,7 @@ var ( EIP3860Epoch: EpochTBD, PragueEpoch: EpochTBD, EIP8024Epoch: EpochTBD, + RejectDuplicateSlashEvidenceEpoch: EpochTBD, } // LocalnetChainConfig contains the chain parameters to run for local development. @@ -391,6 +396,7 @@ var ( EIP3860Epoch: EpochTBD, PragueEpoch: EpochTBD, EIP8024Epoch: EpochTBD, + RejectDuplicateSlashEvidenceEpoch: big.NewInt(0), } // AllProtocolChanges ... @@ -453,6 +459,7 @@ var ( big.NewInt(0), // TimestampValidationEpoch big.NewInt(0), // PragueEpoch big.NewInt(0), // EIP8024Epoch + big.NewInt(0), // RejectDuplicateSlashEvidenceEpoch } // TestChainConfig ... @@ -515,6 +522,7 @@ var ( big.NewInt(0), // TimestampValidationEpoch big.NewInt(0), // PragueEpoch big.NewInt(0), // EIP8024Epoch + big.NewInt(0), // RejectDuplicateSlashEvidenceEpoch } // TestRules ... @@ -726,6 +734,11 @@ type ChainConfig struct { PragueEpoch *big.Int `json:"prague-epoch,omitempty"` // EIP8024Epoch is the first epoch to support EIP-8024 (DUPN, SWAPN, EXCHANGE opcodes) EIP8024Epoch *big.Int `json:"eip8024-epoch,omitempty"` + + // RejectDuplicateSlashEvidenceEpoch is the first epoch where beacon header slash + // payloads are validated with stricter canonical uniqueness rules. Until set to a + // concrete epoch on a network, EpochTBD leaves the rule inactive there. + RejectDuplicateSlashEvidenceEpoch *big.Int `json:"reject-duplicate-slash-evidence-epoch,omitempty"` } // String implements the fmt.Stringer interface. @@ -1012,6 +1025,12 @@ func (c *ChainConfig) IsEIP8024(epoch *big.Int) bool { return isForked(c.EIP8024Epoch, epoch) } +// IsRejectDuplicateSlashEvidence returns whether stricter beacon header slash +// payload checks are active for the given epoch. +func (c *ChainConfig) IsRejectDuplicateSlashEvidence(epoch *big.Int) bool { + return isForked(c.RejectDuplicateSlashEvidenceEpoch, epoch) +} + // IsChainIdFix returns whether epoch is either equal to the ChainId Fix fork epoch or greater. func (c *ChainConfig) IsChainIdFix(epoch *big.Int) bool { return isForked(c.ChainIdFixEpoch, epoch) diff --git a/node/harmony/worker/slash_duplicate_reporter_test.go b/node/harmony/worker/slash_duplicate_reporter_test.go new file mode 100644 index 0000000000..5bd9b7962d --- /dev/null +++ b/node/harmony/worker/slash_duplicate_reporter_test.go @@ -0,0 +1,270 @@ +package worker + +import ( + "fmt" + "math/big" + "testing" + + "github.com/ethereum/go-ethereum/common" + bls_core "github.com/harmony-one/bls/ffi/go/bls" + "github.com/harmony-one/harmony/block" + blockfactory "github.com/harmony-one/harmony/block/factory" + consensus_sig "github.com/harmony-one/harmony/consensus/signature" + "github.com/harmony-one/harmony/core" + "github.com/harmony-one/harmony/core/rawdb" + "github.com/harmony-one/harmony/core/state" + "github.com/harmony-one/harmony/core/types" + "github.com/harmony-one/harmony/core/vm" + "github.com/harmony-one/harmony/crypto/bls" + "github.com/harmony-one/harmony/crypto/hash" + chain2 "github.com/harmony-one/harmony/internal/chain" + "github.com/harmony-one/harmony/internal/params" + "github.com/harmony-one/harmony/numeric" + "github.com/harmony-one/harmony/shard" + "github.com/harmony-one/harmony/staking/effective" + "github.com/harmony-one/harmony/staking/slash" + staking "github.com/harmony-one/harmony/staking/types" + "github.com/stretchr/testify/require" +) + +const ( + slashTestEpoch = int64(2) + slashTestBlockNumber = uint64(37) + slashTestViewID = uint64(38) +) + +// TestVerifySlashes_RejectsReporterVariantDuplicateEvidence is a regression test for +// bug07: duplicate evidence with different reporters must not reach successes. +func TestVerifySlashes_RejectsReporterVariantDuplicateEvidence(t *testing.T) { + offenderKey := newSlashTestBLSKeyPair(t) + offenderAddr := slashTestAddress("offender") + reporterA := slashTestAddress("reporter-a") + reporterB := slashTestAddress("reporter-b") + + chain, _ := newSlashVerifyBeaconChain(t, offenderKey, offenderAddr) + headState, err := chain.State() + require.NoError(t, err) + require.NoError(t, slash.Verify(chain, headState, &slash.Record{ + Evidence: slashTestEvidence(t, offenderKey, offenderAddr), + Reporter: reporterA, + })) + + evidence := slashTestEvidence(t, offenderKey, offenderAddr) + records := slash.Records{ + {Evidence: evidence, Reporter: reporterA}, + {Evidence: evidence, Reporter: reporterB}, + } + + w := New(chain, chain) + successes, failures := w.verifySlashes(records) + + require.Len(t, successes, 1, "only one evidence payload should be accepted") + require.Len(t, failures, 1, "reporter-variant clone must be rejected as duplicate evidence") + require.Equal(t, reporterB, failures[0].Reporter) + require.Equal(t, hash.FromRLPNew256(evidence), hash.FromRLPNew256(successes[0].Evidence)) +} + +func newSlashVerifyBeaconChain( + t *testing.T, + offenderKey slashTestBLSKeyPair, + offenderAddr common.Address, +) (*core.BlockChainImpl, *state.DB) { + t.Helper() + + slashEpoch := big.NewInt(slashTestEpoch) + genesisCommittee := slashTestCommittee(common.Big0, offenderAddr, offenderKey.pub) + committeeAtSlashEpoch := slashTestCommittee(slashEpoch, offenderAddr, offenderKey.pub) + + db := rawdb.NewMemoryDatabase() + gspec := core.Genesis{ + Config: params.TestChainConfig, + Factory: blockfactory.ForTest, + ShardID: shard.BeaconChainShardID, + GasLimit: params.TestGenesisGasLimit, + ShardState: genesisCommittee, + } + genesis := gspec.MustCommit(db) + + encodedCommittee, err := shard.EncodeWrapper(committeeAtSlashEpoch, true) + require.NoError(t, err) + require.NoError(t, rawdb.WriteShardStateBytes(db, slashEpoch, encodedCommittee)) + + chain, err := core.NewBlockChain( + db, nil, nil, &core.CacheConfig{SnapshotLimit: 0}, gspec.Config, chain2.NewEngine(), vm.Config{}, + ) + require.NoError(t, err) + + statedb, err := chain.StateAt(genesis.Root()) + require.NoError(t, err) + + validator := slashTestValidator(offenderAddr, offenderKey.pub, slashEpoch) + require.NoError(t, statedb.UpdateValidatorWrapper(offenderAddr, validator)) + require.NoError(t, chain.WriteValidatorSnapshot( + statedb.Database().DiskDB(), + &staking.ValidatorSnapshot{ + Validator: validator, + Epoch: new(big.Int).Set(slashEpoch), + }, + )) + + processor := core.NewStateProcessor(chain, chain) + head := slashTestHeader(slashEpoch, 1, slashTestViewID, 1, genesis.Hash(), offenderAddr) + inputBlock := types.NewBlockWithHeader(head) + receipts, outcxs, stakeMsgs, _, usedGas, payout, _, err := processor.Process( + inputBlock, statedb, vm.Config{}, false, + ) + require.NoError(t, err) + + head.SetGasUsed(usedGas) + head.SetRoot(statedb.IntermediateRoot(params.TestChainConfig.IsS3(head.Epoch()))) + finalBlock := types.NewBlock(head, nil, receipts, outcxs, nil, nil) + status, err := chain.WriteBlockWithState(finalBlock, receipts, outcxs, stakeMsgs, payout, statedb) + require.NoError(t, err) + require.Equal(t, core.CanonStatTy, status) + + return chain, statedb +} + +func slashTestEvidence( + t *testing.T, + offenderKey slashTestBLSKeyPair, + offenderAddr common.Address, +) slash.Evidence { + t.Helper() + + firstBlock := slashTestBlock(big.NewInt(slashTestEpoch), slashTestBlockNumber, slashTestViewID, 0, common.HexToHash("0x01"), offenderAddr) + secondBlock := slashTestBlock(big.NewInt(slashTestEpoch), slashTestBlockNumber, slashTestViewID, 1, common.HexToHash("0x02"), offenderAddr) + require.NotEqual(t, firstBlock.Hash(), secondBlock.Hash()) + + return slash.Evidence{ + ConflictingVotes: slash.ConflictingVotes{ + FirstVote: slashTestVote(offenderKey, firstBlock), + SecondVote: slashTestVote(offenderKey, secondBlock), + }, + Moment: slash.Moment{ + Epoch: big.NewInt(slashTestEpoch), + ShardID: shard.BeaconChainShardID, + Height: slashTestBlockNumber, + ViewID: slashTestViewID, + }, + Offender: offenderAddr, + } +} + +func slashTestCommittee( + epoch *big.Int, + offenderAddr common.Address, + offenderPub bls.SerializedPublicKey, +) shard.State { + stake := numeric.OneDec() + return shard.State{ + Epoch: new(big.Int).Set(epoch), + Shards: []shard.Committee{{ + ShardID: shard.BeaconChainShardID, + Slots: shard.SlotList{{ + EcdsaAddress: offenderAddr, + BLSPublicKey: offenderPub, + EffectiveStake: &stake, + }}, + }}, + } +} + +func slashTestValidator( + offender common.Address, + offenderPub bls.SerializedPublicKey, + epoch *big.Int, +) *staking.ValidatorWrapper { + selfStake := new(big.Int).Mul(big.NewInt(10000), big.NewInt(1e18)) + return &staking.ValidatorWrapper{ + Validator: staking.Validator{ + Address: offender, + SlotPubKeys: []bls.SerializedPublicKey{offenderPub}, + LastEpochInCommittee: new(big.Int).Add(epoch, common.Big1), + MinSelfDelegation: new(big.Int).Set(selfStake), + MaxTotalDelegation: new(big.Int).Mul(selfStake, common.Big2), + Status: effective.Active, + Commission: staking.Commission{ + CommissionRates: staking.CommissionRates{ + Rate: numeric.MustNewDecFromStr("0.1"), + MaxRate: numeric.MustNewDecFromStr("0.9"), + MaxChangeRate: numeric.MustNewDecFromStr("0.1"), + }, + UpdateHeight: common.Big0, + }, + Description: staking.Description{ + Name: "offender", Identity: "offender", Website: "offender", + SecurityContact: "offender", Details: "offender", + }, + CreationHeight: common.Big0, + }, + Delegations: staking.Delegations{{ + DelegatorAddress: offender, + Amount: new(big.Int).Set(selfStake), + }}, + } +} + +func slashTestHeader( + epoch *big.Int, + number uint64, + viewID uint64, + rootIndex int, + parent common.Hash, + coinbase common.Address, +) *block.Header { + return blockfactory.ForTest.NewHeader(epoch).With(). + Number(new(big.Int).SetUint64(number)). + Epoch(epoch). + ShardID(shard.BeaconChainShardID). + ViewID(new(big.Int).SetUint64(viewID)). + Coinbase(coinbase). + ParentHash(parent). + Root(common.BigToHash(big.NewInt(int64(rootIndex)))). + GasLimit(params.TestGenesisGasLimit). + Header() +} + +func slashTestBlock( + epoch *big.Int, + number uint64, + viewID uint64, + rootIndex int, + parent common.Hash, + coinbase common.Address, +) *types.Block { + return types.NewBlockWithHeader(slashTestHeader(epoch, number, viewID, rootIndex, parent, coinbase)) +} + +func slashTestVote(kp slashTestBLSKeyPair, block *types.Block) slash.Vote { + payload := consensus_sig.ConstructCommitPayload( + params.TestChainConfig, + block.Epoch(), + block.Hash(), + block.NumberU64(), + block.Header().ViewID().Uint64(), + ) + return slash.Vote{ + SignerPubKeys: []bls.SerializedPublicKey{kp.pub}, + BlockHeaderHash: block.Hash(), + Signature: kp.pri.SignHash(payload).Serialize(), + } +} + +type slashTestBLSKeyPair struct { + pri *bls_core.SecretKey + pub bls.SerializedPublicKey +} + +func newSlashTestBLSKeyPair(t *testing.T) slashTestBLSKeyPair { + t.Helper() + pri := bls.RandPrivateKey() + pubKey := pri.GetPublicKey() + var pub bls.SerializedPublicKey + copy(pub[:], pubKey.Serialize()) + return slashTestBLSKeyPair{pri: pri, pub: pub} +} + +func slashTestAddress(label string) common.Address { + return common.BytesToAddress([]byte(fmt.Sprintf("harmony.slash.%s", label))) +} diff --git a/node/harmony/worker/worker.go b/node/harmony/worker/worker.go index 0785541f4c..6930e7bea8 100644 --- a/node/harmony/worker/worker.go +++ b/node/harmony/worker/worker.go @@ -492,12 +492,13 @@ func (w *Worker) verifySlashes( utils.Logger().Warn(). Interface("slashRecord1", existing). Interface("slashRecord2", d[i]). - Msg("Duplicate slash records with different reporters") + Msg("Skipping duplicate slash evidence in slash candidate batch") failures = append(failures, d[i]) + continue } else { seenEvidences[evidenceHash] = struct{}{} - // In addition, need to count the same evidence with first and second vote swapped as seen + // Register an alternate canonical encoding so equivalent evidence is not double-counted swapVote := d[i].Evidence tmp := swapVote.ConflictingVotes.FirstVote swapVote.ConflictingVotes.FirstVote = swapVote.ConflictingVotes.SecondVote