Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: sync with upstream #8

Merged
merged 10 commits into from
Nov 6, 2024
Merged
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
- `[consensus]` Do not panic if the validator index of a `Vote` message is out
of bounds, when vote extensions are enabled
([\#ABC-0021](https://github.com/cometbft/cometbft/security/advisories/GHSA-p7mv-53f2-4cwj))
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
- `[p2p]` fix exponential backoff logic to increase reconnect retries close to 24 hours
([\#3519](https://github.com/cometbft/cometbft/issues/3519))
6 changes: 3 additions & 3 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ jobs:
steps:
- uses: actions/setup-go@v5
with:
go-version: "1.22"
go-version: "1.23"
- uses: actions/checkout@v4
- uses: technote-space/get-diff-action@v6
with:
Expand All @@ -43,7 +43,7 @@ jobs:
steps:
- uses: actions/setup-go@v5
with:
go-version: "1.22"
go-version: "1.23"
- uses: actions/checkout@v4
- uses: technote-space/get-diff-action@v6
with:
Expand All @@ -65,7 +65,7 @@ jobs:
steps:
- uses: actions/setup-go@v5
with:
go-version: "1.22"
go-version: "1.23"
- uses: actions/checkout@v4
- uses: technote-space/get-diff-action@v6
with:
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/check-generated.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ jobs:
steps:
- uses: actions/setup-go@v5
with:
go-version: "1.22"
go-version: "1.23"

- uses: actions/checkout@v4

Expand All @@ -44,7 +44,7 @@ jobs:
steps:
- uses: actions/setup-go@v5
with:
go-version: "1.22"
go-version: "1.23"

- uses: actions/checkout@v4
with:
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/e2e-manual-multiversion.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,13 @@ jobs:
strategy:
fail-fast: false
matrix:
group: ['00', '01', '02', '03', '04', '05']
group: ["00", "01", "02", "03", "04", "05"]
runs-on: ubuntu-latest
timeout-minutes: 60
steps:
- uses: actions/setup-go@v5
with:
go-version: '1.22'
go-version: "1.23"

- uses: actions/checkout@v4

Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/e2e-manual.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,13 @@ jobs:
strategy:
fail-fast: false
matrix:
group: ['00', '01', '02', '03', '04', '05']
group: ["00", "01", "02", "03", "04", "05"]
runs-on: ubuntu-latest
timeout-minutes: 60
steps:
- uses: actions/setup-go@v5
with:
go-version: '1.22'
go-version: "1.23"

- uses: actions/checkout@v4

Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/e2e.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ name: e2e
# Runs the CI end-to-end test network on all pushes to v0.38.x
# and every pull request, but only if any Go files have been changed.
on:
workflow_dispatch: # allow running workflow manually
workflow_dispatch: # allow running workflow manually
pull_request:
push:
branches:
Expand All @@ -15,7 +15,7 @@ jobs:
steps:
- uses: actions/setup-go@v5
with:
go-version: '1.22'
go-version: "1.23"
- uses: actions/checkout@v4
- uses: technote-space/get-diff-action@v6
with:
Expand Down
6 changes: 3 additions & 3 deletions .github/workflows/fuzz-nightly.yml
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
# Runs fuzzing nightly.
name: Fuzz Tests
on:
workflow_dispatch: # allow running workflow manually
workflow_dispatch: # allow running workflow manually
schedule:
- cron: '0 3 * * *'
- cron: "0 3 * * *"
pull_request:
branches:
- v0.38.x
Expand All @@ -16,7 +16,7 @@ jobs:
steps:
- uses: actions/setup-go@v5
with:
go-version: '1.22'
go-version: "1.23"

- uses: actions/checkout@v4

Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/govulncheck.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ jobs:
steps:
- uses: actions/setup-go@v5
with:
go-version: "1.22"
go-version: "1.23"
check-latest: true
- uses: actions/checkout@v4
- uses: technote-space/get-diff-action@v6
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ jobs:
- uses: actions/checkout@v4
- uses: actions/setup-go@v5
with:
go-version: '1.22'
go-version: "1.23"
- uses: technote-space/get-diff-action@v6
with:
PATTERNS: |
Expand Down
8 changes: 4 additions & 4 deletions .github/workflows/pre-release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@ name: "Pre-release"
on:
push:
tags:
- "v[0-9]+.[0-9]+.[0-9]+-alpha.[0-9]+" # e.g. v0.37.0-alpha.1, v0.38.0-alpha.10
- "v[0-9]+.[0-9]+.[0-9]+-beta.[0-9]+" # e.g. v0.37.0-beta.1, v0.38.0-beta.10
- "v[0-9]+.[0-9]+.[0-9]+-rc[0-9]+" # e.g. v0.37.0-rc1, v0.38.0-rc10
- "v[0-9]+.[0-9]+.[0-9]+-alpha.[0-9]+" # e.g. v0.37.0-alpha.1, v0.38.0-alpha.10
- "v[0-9]+.[0-9]+.[0-9]+-beta.[0-9]+" # e.g. v0.37.0-beta.1, v0.38.0-beta.10
- "v[0-9]+.[0-9]+.[0-9]+-rc[0-9]+" # e.g. v0.37.0-rc1, v0.38.0-rc10

jobs:
prerelease:
Expand All @@ -18,7 +18,7 @@ jobs:

- uses: actions/setup-go@v5
with:
go-version: '1.22'
go-version: "1.23"

# Similar check to ./release-version.yml, but enforces this when pushing
# tags. The ./release-version.yml check can be bypassed and is mainly
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/proto-lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ jobs:
timeout-minutes: 5
steps:
- uses: actions/checkout@v4
- uses: bufbuild/buf-setup-action@v1.45.0
- uses: bufbuild/buf-setup-action@v1.46.0
- uses: bufbuild/buf-lint-action@v1
with:
input: 'proto'
4 changes: 2 additions & 2 deletions .github/workflows/release-version.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ name: Check release version
on:
push:
branches:
- 'release/**'
- "release/**"

jobs:
check-version:
Expand All @@ -15,7 +15,7 @@ jobs:

- uses: actions/setup-go@v5
with:
go-version: '1.22'
go-version: "1.23"

- name: Check version
run: |
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ name: "Release"
on:
push:
tags:
- "v[0-9]+.[0-9]+.[0-9]+" # Push events to matching v*, i.e. v1.0, v20.15.10
- "v[0-9]+.[0-9]+.[0-9]+" # Push events to matching v*, i.e. v1.0, v20.15.10

jobs:
release:
Expand All @@ -16,7 +16,7 @@ jobs:

- uses: actions/setup-go@v5
with:
go-version: '1.22'
go-version: "1.23"

# Similar check to ./release-version.yml, but enforces this when pushing
# tags. The ./release-version.yml check can be bypassed and is mainly
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ jobs:
steps:
- uses: actions/setup-go@v5
with:
go-version: "1.22"
go-version: "1.23"
- uses: actions/checkout@v4
- uses: technote-space/get-diff-action@v6
with:
Expand Down
2 changes: 1 addition & 1 deletion DOCKER/Dockerfile
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# Use a build arg to ensure that both stages use the same,
# hopefully current, go version.
ARG GOLANG_BASE_IMAGE=golang:1.22-alpine
ARG GOLANG_BASE_IMAGE=golang:1.23-alpine

# stage 1 Generate CometBFT Binary
FROM --platform=$BUILDPLATFORM $GOLANG_BASE_IMAGE as builder
Expand Down
9 changes: 9 additions & 0 deletions consensus/errors.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package consensus

type ErrInvalidVote struct {
Reason string
}

func (e ErrInvalidVote) Error() string {
return "invalid vote: " + e.Reason
}
37 changes: 37 additions & 0 deletions consensus/reactor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"github.com/cometbft/cometbft/libs/bytes"
"github.com/cometbft/cometbft/libs/json"
"github.com/cometbft/cometbft/libs/log"
cmtrand "github.com/cometbft/cometbft/libs/rand"
cmtsync "github.com/cometbft/cometbft/libs/sync"
mempl "github.com/cometbft/cometbft/mempool"
"github.com/cometbft/cometbft/p2p"
Expand Down Expand Up @@ -1126,3 +1127,39 @@ func TestMarshalJSONPeerState(t *testing.T) {
"block_parts":"0"}
}`, string(data))
}

func TestVoteMessageValidateBasic(t *testing.T) {
_, vss := randState(2)

randBytes := cmtrand.Bytes(tmhash.Size)
blockID := types.BlockID{
Hash: randBytes,
PartSetHeader: types.PartSetHeader{
Total: 1,
Hash: randBytes,
},
}
vote := signVote(vss[1], cmtproto.PrecommitType, randBytes, blockID.PartSetHeader, true)

testCases := []struct {
malleateFn func(*VoteMessage)
expErr string
}{
{func(_ *VoteMessage) {}, ""},
{func(msg *VoteMessage) { msg.Vote.ValidatorIndex = -1 }, "negative ValidatorIndex"},
// INVALID, but passes ValidateBasic, since the method does not know the number of active validators
{func(msg *VoteMessage) { msg.Vote.ValidatorIndex = 1000 }, ""},
}

for i, tc := range testCases {
t.Run(fmt.Sprintf("#%d", i), func(t *testing.T) {
msg := &VoteMessage{vote}

tc.malleateFn(msg)
err := msg.ValidateBasic()
if tc.expErr != "" && assert.Error(t, err) { //nolint:testifylint // require.Error doesn't work with the conditional here
assert.Contains(t, err.Error(), tc.expErr)
}
})
}
}
8 changes: 8 additions & 0 deletions consensus/state.go
Original file line number Diff line number Diff line change
Expand Up @@ -2196,6 +2196,14 @@ func (cs *State) addVote(vote *types.Vote, peerID p2p.ID) (added bool, err error
// Here, we verify the signature of the vote extension included in the vote
// message.
_, val := cs.state.Validators.GetByIndex(vote.ValidatorIndex)
if val == nil { // TODO: we should disconnect from this malicious peer
valsCount := cs.state.Validators.Size()
cs.Logger.Info("Peer sent us vote with invalid ValidatorIndex",
"peer", peerID,
"validator_index", vote.ValidatorIndex,
"len_validators", valsCount)
return added, ErrInvalidVote{Reason: fmt.Sprintf("ValidatorIndex %d is out of bounds [0, %d)", vote.ValidatorIndex, valsCount)}
}
if err := vote.VerifyExtension(cs.state.ChainID, val.PubKey); err != nil {
return false, err
}
Expand Down
27 changes: 27 additions & 0 deletions consensus/state_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1938,6 +1938,33 @@ func TestVoteExtensionEnableHeight(t *testing.T) {
}
}

// TestStateDoesntCrashOnInvalidVote tests that the state does not crash when
// receiving an invalid vote. In particular, one with the incorrect
// ValidatorIndex.
func TestStateDoesntCrashOnInvalidVote(t *testing.T) {
cs, vss := randState(2)
height, round := cs.Height, cs.Round
// create dummy peer
peer := p2pmock.NewPeer(nil)

startTestRound(cs, height, round)

vote := signVote(vss[1], cmtproto.PrecommitType, nil, types.PartSetHeader{}, true)
// Non-existent validator index
vote.ValidatorIndex = int32(len(vss))

voteMessage := &VoteMessage{vote}
assert.NotPanics(t, func() {
cs.handleMsg(msgInfo{voteMessage, peer.ID()})
})

added, err := cs.AddVote(vote, peer.ID())
assert.False(t, added)
assert.NoError(t, err)
// TODO: uncomment once we punish peer and return an error
// assert.Equal(t, ErrInvalidVote{Reason: "ValidatorIndex 2 is out of bounds [0, 2)"}, err)
}

// 4 vals, 3 Nil Precommits at P0
// What we want:
// P0 waits for timeoutPrecommit before starting next round
Expand Down
Loading
Loading