Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .github/workflows/claude-code-review.yml
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,8 @@ jobs:
memcan_url: ${{ vars.MEMCAN_URL }}
memcan_api_key: ${{ secrets.MEMCAN_API_KEY }}
claude_extra_args: "--permission-mode auto"
upload_transcripts: "true"
transcripts_recipients: "github:lklimek"
# Claudius-Maginificent only has triage access to this repo (no git
# push), so the collaborator-permission API reports it as "read" —
# below claude-code-action's admin/write bar. It's a trusted bot
Expand Down
11 changes: 11 additions & 0 deletions internal/consensus/state_data.go
Original file line number Diff line number Diff line change
Expand Up @@ -686,5 +686,16 @@ func (s *StateData) isValidForPrevote() error {
if err := sm.ValidateBlockChainLock(s.state, s.ProposalBlock); err != nil {
return errPrevoteInvalidChainLock
}
// Reproposals retain the original header; their POL is checked before voting.
if s.Proposal.POLRound == -1 {
proposer, err := s.ProposerSelector.GetProposer(s.Height, s.Round)
if err != nil {
return fmt.Errorf("get proposal proposer: %w", err)
}
if !bytes.Equal(s.ProposalBlock.ProposerProTxHash, proposer.ProTxHash) {
return fmt.Errorf("proposal-block proposer %X does not match round proposer %X",
s.ProposalBlock.ProposerProTxHash, proposer.ProTxHash)
Comment on lines +695 to +697

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Suggestion: Use a sentinel for fresh-proposal proposer mismatch

The new proposer-mismatch branch returns an anonymous formatted error, while the other prevote validation failures use package sentinel errors. This prevents callers and tests from matching the rejection with errors.Is and makes the new consensus-critical failure reason less consistent and observable than its neighboring checks. Define an errPrevoteInvalidProposer sentinel and wrap it in this message; the nil-prevote behavior remains unchanged.

source: muse-spark-1.3-contributor (phase1-reviewer: general)

}
}
return nil
}
7 changes: 7 additions & 0 deletions internal/consensus/state_data_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"github.com/stretchr/testify/require"

cstypes "github.com/dashpay/tenderdash/internal/consensus/types"
"github.com/dashpay/tenderdash/internal/consensus/versioned/selectproposer"
sm "github.com/dashpay/tenderdash/internal/state"
"github.com/dashpay/tenderdash/internal/test/factory"
tmrequire "github.com/dashpay/tenderdash/internal/test/require"
Expand Down Expand Up @@ -106,6 +107,12 @@ func TestIsValidForPrevote(t *testing.T) {
state: tc.state,
RoundState: tc.rs,
}
selector, err := selectproposer.NewHeightRoundProposerSelector(valSet.Copy(), tc.rs.Height, tc.rs.Round, nil, nil)
require.NoError(t, err)
stateData.ProposerSelector = selector
if stateData.ProposalBlock != nil {
stateData.ProposalBlock.ProposerProTxHash = valSet.Proposer().ProTxHash
}
tmrequire.Error(t, tc.wantErr, stateData.isValidForPrevote())
})
}
Expand Down
73 changes: 71 additions & 2 deletions internal/consensus/state_prevoter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"github.com/stretchr/testify/suite"

cstypes "github.com/dashpay/tenderdash/internal/consensus/types"
"github.com/dashpay/tenderdash/internal/consensus/versioned/selectproposer"
sm "github.com/dashpay/tenderdash/internal/state"
"github.com/dashpay/tenderdash/internal/state/mocks"
"github.com/dashpay/tenderdash/internal/test/factory"
Expand Down Expand Up @@ -196,6 +197,70 @@ func (suite *PrevoterTestSuite) TestCheckProposalBlock() {
}
}

func (suite *PrevoterTestSuite) TestDoProposerProTxHash() {
for _, tc := range []struct {
name string
round int32
polRound int32
wrong bool
pol bool
otherBlock bool
wantVote bool
}{
{name: "new block round zero", polRound: -1, wantVote: true},
{name: "new block later round", round: 1, polRound: -1, wantVote: true},
{name: "wrong proposer round zero", polRound: -1, wrong: true},
{name: "predecessor capture", round: 1, polRound: -1, wrong: true},
{name: "reproposal retains original proposer", round: 1, polRound: 0, wrong: true, pol: true, wantVote: true},
{name: "claimed POL without votes", round: 1, polRound: 0, wrong: true},
{name: "POL for another block", round: 1, polRound: 0, wrong: true, pol: true, otherBlock: true},
} {
suite.Run(tc.name, func() {
suite.SetupTest()
stateData := suite.makeValidStateData()
stateData.Round = tc.round
stateData.Proposal.Round = tc.round
stateData.Proposal.POLRound = tc.polRound
proposer, err := stateData.ProposerSelector.GetProposer(stateData.Height, tc.round)
suite.Require().NoError(err)
stateData.ProposalBlock.ProposerProTxHash = proposer.ProTxHash
if tc.wrong {
index, _ := suite.valSet.GetByProTxHash(proposer.ProTxHash)
size := int32(suite.valSet.Size())
stateData.ProposalBlock.ProposerProTxHash = suite.valSet.GetByIndex((index + size - 1) % size).ProTxHash
}
stateData.Proposal.BlockID = types.BlockID{Hash: stateData.ProposalBlock.Hash()}
if tc.pol {
stateData.Votes = suite.makeHeightVoteSetMaj23(stateData, tc.polRound)
}
if tc.otherBlock {
stateData.ProposalBlock.ProposedAppVersion++
stateData.Proposal.BlockID = types.BlockID{Hash: stateData.ProposalBlock.Hash()}
}
rejectBeforeProcess := tc.wrong && tc.polRound == -1
if !rejectBeforeProcess {
suite.mockExecutor.On("ProcessProposal", mock.Anything, mock.Anything, tc.round,
mock.Anything, true, types.VerifiedCommit{}).Once().Return(sm.CurrentRoundState{}, nil)
suite.mockExecutor.On("ValidateBlockWithRoundState", mock.Anything, mock.Anything,
mock.Anything, mock.Anything, mock.Anything).Once().Return(nil)
}
wantBlockID := types.BlockID{}
if tc.wantVote {
wantBlockID = stateData.Proposal.BlockID
}
suite.mockWAL.On("FlushAndSync").Return(nil)
suite.mockQueueSender.On("send", mock.Anything, mock.MatchedBy(func(msg *VoteMessage) bool {
return msg.Vote.Type == tmproto.PrevoteType && msg.Vote.BlockID.Equals(wantBlockID)
}), types.NodeID("")).Once().Return(nil)
suite.Require().NoError(suite.prevoter.Do(context.Background(), &stateData))
if rejectBeforeProcess {
suite.mockExecutor.AssertNotCalled(suite.T(), "ProcessProposal", mock.Anything, mock.Anything,
mock.Anything, mock.Anything, mock.Anything, mock.Anything)
}
})
}
}

func (suite *PrevoterTestSuite) makeHeightVoteSetMaj23(stateData StateData, round int32) *cstypes.HeightVoteSet {
ctx := context.Background()
vote1 := types.Vote{
Expand Down Expand Up @@ -300,8 +365,9 @@ func (suite *PrevoterTestSuite) makeValidStateData() StateData {
}
block := &types.Block{
Header: types.Header{
Time: now,
ValidatorsHash: []byte{1, 2, 3, 4},
Time: now,
ValidatorsHash: []byte{1, 2, 3, 4},
ProposerProTxHash: suite.valSet.Proposer().ProTxHash,
},
Data: types.Data{
Txs: types.Txs{[]byte{1}, []byte{2}},
Expand All @@ -323,6 +389,9 @@ func (suite *PrevoterTestSuite) makeValidStateData() StateData {
Validators: suite.valSet,
Votes: cstypes.NewHeightVoteSet("test-chain", 1000, suite.valSet),
}
selector, err := selectproposer.NewHeightRoundProposerSelector(suite.valSet.Copy(), 1000, 0, nil, suite.logger)
suite.Require().NoError(err)
validRoundState.ProposerSelector = selector
return StateData{
state: validState,
RoundState: validRoundState,
Expand Down
7 changes: 5 additions & 2 deletions internal/consensus/state_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1635,11 +1635,12 @@ func TestStateLock_POLSafety2(t *testing.T) {
// the block for round 1
// We add some tx so that the proposal will differ from the round 0 one
// We cannot rely on time because blocks at initial height have genesis time
mpool := (cs1.txNotifier).(mempool.Mempool)
cs2 := newState(ctx, t, cs1.logger, stateData.state, vs2, newKVStoreFunc(t)(cs1.logger, ""))
mpool := (cs2.txNotifier).(mempool.Mempool)
err = mpool.CheckTx(ctx, types.Tx("round1"), nil, mempool.TxInfo{})
assert.NoError(t, err)

prop1, propBlock1 := decideProposal(ctx, t, cs1, vs2, vs2.Height, vs2.Round+1)
prop1, propBlock1 := decideProposal(ctx, t, cs2, vs2, vs2.Height, vs2.Round+1)
propBlockParts1, err := propBlock1.MakePartSet(partSize)
require.NoError(t, err)
propBlockID1 := propBlock1.BlockID(propBlockParts1)
Expand Down Expand Up @@ -3376,6 +3377,8 @@ func TestStateTimestamp_ProposalMatch(t *testing.T) {
round++
incrementRound(vss[1:]...)

propBlock.ProposerProTxHash, err = vs2.GetProTxHash(ctx)
require.NoError(t, err)
propBlockParts, err := propBlock.MakePartSet(types.BlockPartSizeBytes)
require.NoError(t, err)
blockID := propBlock.BlockID(propBlockParts)
Expand Down
Loading