From 31a155dc0496ab0e4f797d3a736cc799330e22a9 Mon Sep 17 00:00:00 2001 From: Nate Maninger Date: Thu, 6 Jun 2024 08:01:19 -0700 Subject: [PATCH 1/6] gateway: revert memory reuse --- gateway/encoding.go | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/gateway/encoding.go b/gateway/encoding.go index 0e40f628..cc74fec5 100644 --- a/gateway/encoding.go +++ b/gateway/encoding.go @@ -247,11 +247,7 @@ func (r *RPCSendBlocks) encodeResponse(e *types.Encoder) { } } func (r *RPCSendBlocks) decodeResponse(d *types.Decoder) { - if n := d.ReadPrefix(); n <= cap(r.Blocks) { - r.Blocks = r.Blocks[:n] - } else { - r.Blocks = make([]types.Block, n) - } + r.Blocks = make([]types.Block, d.ReadPrefix()) for i := range r.Blocks { (*types.V1Block)(&r.Blocks[i]).DecodeFrom(d) } From 9ec8e497dd9bf2089fd0852e962dece4da461727 Mon Sep 17 00:00:00 2001 From: Nate Maninger Date: Thu, 6 Jun 2024 08:01:32 -0700 Subject: [PATCH 2/6] types: clear decoder buffer when read encounters an error --- types/encoding.go | 11 ++++++++-- types/encoding_test.go | 46 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 2 deletions(-) create mode 100644 types/encoding_test.go diff --git a/types/encoding.go b/types/encoding.go index e847f84b..ee281489 100644 --- a/types/encoding.go +++ b/types/encoding.go @@ -136,8 +136,15 @@ func (d *Decoder) Read(p []byte) (int, error) { if want > len(d.buf) { want = len(d.buf) } - var read int - read, d.err = io.ReadFull(&d.lr, d.buf[:want]) + read, err := io.ReadFull(&d.lr, d.buf[:want]) + if err != nil { + // When the decoder encounters an error, it must clear its buffer and + // return empty values for the remaining decodes. Because the decoder uses + // sticky errors instead of immediately returning a bad decode can + // potentially cause a massive allocation. + d.SetErr(err) + return n, err + } n += copy(p[n:], d.buf[:read]) } return n, d.err diff --git a/types/encoding_test.go b/types/encoding_test.go new file mode 100644 index 00000000..1f0d8638 --- /dev/null +++ b/types/encoding_test.go @@ -0,0 +1,46 @@ +package types + +import ( + "io" + "testing" +) + +type readErrorer struct { + err error + r io.Reader +} + +func (r *readErrorer) Read(p []byte) (n int, err error) { + if r.err != nil { + return 0, r.err + } + return r.r.Read(p) +} + +func TestDecoderError(t *testing.T) { + r, w := io.Pipe() + re := &readErrorer{r: r} + enc := NewEncoder(w) + d := NewDecoder(io.LimitedReader{R: re, N: 1e6}) + + go func() { + // writing to the pipe blocks until we read from it + enc.WritePrefix(1000) + enc.Flush() + }() + + // read the value from the encoder + n := d.ReadPrefix() + if n != 1000 { + t.Fatalf("expected 1000, got %d", n) + } + + // set the error and try to read again + re.err = io.EOF + + // should return 0 since the decoder errored + n = d.ReadPrefix() + if n != 0 { + t.Fatalf("expected 0, got %d", n) + } +} From 9d2a1575b84c6852d5c36c2c25b8540a3bf5df13 Mon Sep 17 00:00:00 2001 From: Nate Maninger Date: Thu, 6 Jun 2024 09:20:40 -0700 Subject: [PATCH 3/6] consensus: lint suggestions --- consensus/validation.go | 2 +- consensus/validation_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/consensus/validation.go b/consensus/validation.go index e9213ddc..147d8d26 100644 --- a/consensus/validation.go +++ b/consensus/validation.go @@ -153,7 +153,7 @@ func validateCurrencyOverflow(ms *MidState, txn types.Transaction) error { return nil } -func validateMinimumValues(ms *MidState, txn types.Transaction) error { +func validateMinimumValues(_ *MidState, txn types.Transaction) error { zero := false for _, sco := range txn.SiacoinOutputs { zero = zero || sco.Value.IsZero() diff --git a/consensus/validation_test.go b/consensus/validation_test.go index e860603a..89afb164 100644 --- a/consensus/validation_test.go +++ b/consensus/validation_test.go @@ -94,7 +94,7 @@ func (db *consensusDB) supplementTipBlock(b types.Block) (bs V1BlockSupplement) return bs } -func (db *consensusDB) ancestorTimestamp(id types.BlockID) time.Time { +func (db *consensusDB) ancestorTimestamp(types.BlockID) time.Time { return time.Time{} } From 223630e29a37624b6d8b4e987df29d51ea2142b3 Mon Sep 17 00:00:00 2001 From: Nate Maninger Date: Fri, 7 Jun 2024 08:52:50 -0700 Subject: [PATCH 4/6] upgrade Go --- go.mod | 2 +- types/encoding.go | 15 ++------------- 2 files changed, 3 insertions(+), 14 deletions(-) diff --git a/go.mod b/go.mod index c5cb82af..82fcca23 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module go.sia.tech/core -go 1.20 +go 1.21.8 require ( go.sia.tech/mux v1.2.0 diff --git a/types/encoding.go b/types/encoding.go index ee281489..00c707b4 100644 --- a/types/encoding.go +++ b/types/encoding.go @@ -132,20 +132,9 @@ func (d *Decoder) Err() error { return d.err } func (d *Decoder) Read(p []byte) (int, error) { n := 0 for len(p[n:]) > 0 && d.err == nil { - want := len(p[n:]) - if want > len(d.buf) { - want = len(d.buf) - } - read, err := io.ReadFull(&d.lr, d.buf[:want]) - if err != nil { - // When the decoder encounters an error, it must clear its buffer and - // return empty values for the remaining decodes. Because the decoder uses - // sticky errors instead of immediately returning a bad decode can - // potentially cause a massive allocation. - d.SetErr(err) - return n, err - } + read, err := io.ReadFull(&d.lr, d.buf[:min(len(p[n:]), len(d.buf))]) n += copy(p[n:], d.buf[:read]) + d.SetErr(err) } return n, d.err } From 76c87801cc51c0216a1412f429278575dc53221d Mon Sep 17 00:00:00 2001 From: Nate Maninger Date: Fri, 7 Jun 2024 09:05:50 -0700 Subject: [PATCH 5/6] ci: fix golangci-lint, fix linter errors --- .golangci.yml | 66 ++++++++++++++---------------------- consensus/validation_test.go | 10 +++--- types/policy_test.go | 4 +-- 3 files changed, 33 insertions(+), 47 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 0ac74567..72819f2f 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -17,28 +17,8 @@ run: # list of build tags, all linters use it. Default is empty list. build-tags: [] - # which dirs to skip: issues from them won't be reported; - # can use regexp here: generated.*, regexp is applied on full path; - # default value is empty list, but default dirs are skipped independently - # from this option's value (see skip-dirs-use-default). - skip-dirs: - - cover - - # default is true. Enables skipping of directories: - # vendor$, third_party$, testdata$, examples$, Godeps$, builtin$ - skip-dirs-use-default: true - - # which files to skip: they will be analyzed, but issues from them - # won't be reported. Default value is empty list, but there is - # no need to include all autogenerated files, we confidently recognize - # autogenerated files. If it's not please let us know. - skip-files: [] - # output configuration options output: - # colored-line-number|line-number|json|tab|checkstyle|code-climate, default is "colored-line-number" - format: colored-line-number - # print lines of code with issue, default is true print-issued-lines: true @@ -50,7 +30,6 @@ linters-settings: ## Enabled linters: govet: # report about shadowed variables - check-shadowing: false disable-all: false tagliatelle: @@ -65,24 +44,31 @@ linters-settings: # See https://go-critic.github.io/overview#checks-overview # To check which checks are enabled run `GL_DEBUG=gocritic golangci-lint run` # By default list of stable checks is used. - disable-all: true - enabled-checks: - - argOrder # Diagnostic options - - badCond - - caseOrder - - dupArg - - dupBranchBody - - dupCase - - dupSubExpr - - nilValReturn - - offBy1 - - weakCond - - boolExprSimplify # Style options here and below. - - builtinShadow - - emptyFallthrough - - hexLiteral - - underef - - equalFold + enabled-tags: + - diagnostic + - style + disabled-checks: + # diagnostic + - commentedOutCode + - uncheckedInlineErr + + # style + - emptyStringTest + - exitAfterDefer + - ifElseChain + - importShadow + - initClause + - nestingReduce + - octalLiteral + - paramTypeCombine + - ptrToRefParam + - stringsCompare + - tooManyResultsChecker + - typeDefFirst + - typeUnparen + - unlabelStmt + - unnamedResult + - whyNoLint revive: ignore-generated-header: true rules: @@ -91,7 +77,7 @@ linters-settings: - name: bool-literal-in-expr disabled: false - name: confusing-naming - disabled: true + disabled: false - name: confusing-results disabled: false - name: constant-logical-expr diff --git a/consensus/validation_test.go b/consensus/validation_test.go index 89afb164..d3c5403d 100644 --- a/consensus/validation_test.go +++ b/consensus/validation_test.go @@ -409,14 +409,14 @@ func TestValidateBlock(t *testing.T) { "siafund outputs exceed inputs", func(b *types.Block) { txn := &b.Transactions[0] - txn.SiafundOutputs[0].Value = txn.SiafundOutputs[0].Value + 1 + txn.SiafundOutputs[0].Value++ }, }, { "siafund outputs less than inputs", func(b *types.Block) { txn := &b.Transactions[0] - txn.SiafundOutputs[0].Value = txn.SiafundOutputs[0].Value - 1 + txn.SiafundOutputs[0].Value-- }, }, { @@ -591,7 +591,7 @@ func TestValidateBlock(t *testing.T) { func(b *types.Block) { txn := &b.Transactions[0] txn.SiafundInputs = append(txn.SiafundInputs, txn.SiafundInputs[0]) - txn.SiafundOutputs[0].Value = txn.SiafundOutputs[0].Value + 100 + txn.SiafundOutputs[0].Value += 100 }, }, { @@ -965,14 +965,14 @@ func TestValidateV2Block(t *testing.T) { "siafund outputs exceed inputs", func(b *types.Block) { txn := &b.V2.Transactions[0] - txn.SiafundOutputs[0].Value = txn.SiafundOutputs[0].Value + 1 + txn.SiafundOutputs[0].Value++ }, }, { "siafund outputs less than inputs", func(b *types.Block) { txn := &b.V2.Transactions[0] - txn.SiafundOutputs[0].Value = txn.SiafundOutputs[0].Value - 1 + txn.SiafundOutputs[0].Value-- }, }, { diff --git a/types/policy_test.go b/types/policy_test.go index b2df4a08..a652aa11 100644 --- a/types/policy_test.go +++ b/types/policy_test.go @@ -393,14 +393,14 @@ func TestSatisfiedPolicyMarshalJSON(t *testing.T) { name: "PolicyWithSignature", sp: PolicyPublicKey(publicKey), signatures: []Signature{signature}, - exp: fmt.Sprintf(`{"policy":"pk(0x%x)","signatures":["%s"]}`, publicKey[:], signature), + exp: fmt.Sprintf(`{"policy":"pk(0x%x)","signatures":[%q]}`, publicKey[:], signature), }, { name: "PolicyWithSignaturesAndPreimages", sp: PolicyThreshold(1, []SpendPolicy{PolicyPublicKey(publicKey), PolicyHash(hash)}), signatures: []Signature{signature}, preimages: [][]byte{{1, 2, 3}}, - exp: fmt.Sprintf(`{"policy":"thresh(1,[pk(0x%x),h(0x%x)])","signatures":["%s"],"preimages":["010203"]}`, publicKey[:], hash[:], signature), + exp: fmt.Sprintf(`{"policy":"thresh(1,[pk(0x%x),h(0x%x)])","signatures":[%q],"preimages":["010203"]}`, publicKey[:], hash[:], signature), }, { name: "PolicyWithPreimagesOnly", From 5c81d3198dff07873b09595eadf64f32183b3303 Mon Sep 17 00:00:00 2001 From: Nate Maninger Date: Fri, 7 Jun 2024 09:08:40 -0700 Subject: [PATCH 6/6] ci: upgrade workflows --- .github/workflows/nightly.yml | 10 +++++----- .github/workflows/test.yml | 10 +++++----- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/.github/workflows/nightly.yml b/.github/workflows/nightly.yml index e7d4e345..ef14d9b9 100644 --- a/.github/workflows/nightly.yml +++ b/.github/workflows/nightly.yml @@ -10,22 +10,22 @@ jobs: strategy: matrix: os: [ ubuntu-latest, macos-latest, windows-latest ] - go-version: [ '1.20', '1.21' ] + go-version: [ '1.21', '1.22' ] runs-on: ${{ matrix.os }} timeout-minutes: 120 steps: - - uses: actions/checkout@v3 - - uses: actions/setup-go@v3 + - uses: actions/checkout@v4 + - uses: actions/setup-go@v5 with: go-version: ${{ matrix.go-version }} - name: test - uses: n8maninger/action-golang-test@v1 + uses: n8maninger/action-golang-test@v2 with: args: "-count=50;-timeout=30m;-failfast" skip-go-install: true show-package-output: true - name: test-race - uses: n8maninger/action-golang-test@v1 + uses: n8maninger/action-golang-test@v2 with: args: "-race;-count=50;-timeout=30m;-failfast" skip-go-install: true diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index d9a4db4a..66a98f19 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -11,23 +11,23 @@ jobs: strategy: matrix: os: [ ubuntu-latest, macos-latest, windows-latest ] - go-version: [ '1.20', '1.21' ] + go-version: [ '1.21', '1.22' ] runs-on: ${{ matrix.os }} timeout-minutes: 10 steps: - name: Configure git # required for golangci-lint on Windows shell: bash run: git config --global core.autocrlf false - - uses: actions/checkout@v3 - - uses: actions/setup-go@v3 + - uses: actions/checkout@v4 + - uses: actions/setup-go@v5 with: go-version: ${{ matrix.go-version }} - name: Lint - uses: golangci/golangci-lint-action@v3 + uses: golangci/golangci-lint-action@v4 with: skip-cache: true - name: test - uses: n8maninger/action-golang-test@v1 + uses: n8maninger/action-golang-test@v2 with: args: "-race" skip-go-install: true