From 1e30a73a9c475209225d85e3d83a04802d8ec9a3 Mon Sep 17 00:00:00 2001 From: Rod Vagg Date: Thu, 5 Sep 2024 19:21:24 +1000 Subject: [PATCH] fixup! feat!: extract BatchReturn to batch package, add methods & tests --- batch/batch_return.go | 38 ++++++- batch/batch_return_test.go | 224 ++++++++++++++++++++++++++++++------- 2 files changed, 215 insertions(+), 47 deletions(-) diff --git a/batch/batch_return.go b/batch/batch_return.go index 5d570434..983abc78 100644 --- a/batch/batch_return.go +++ b/batch/batch_return.go @@ -1,6 +1,10 @@ package batch -import "github.com/filecoin-project/go-state-types/exitcode" +import ( + "errors" + + "github.com/filecoin-project/go-state-types/exitcode" +) type BatchReturn struct { SuccessCount uint64 @@ -38,11 +42,37 @@ func (b BatchReturn) Codes() []exitcode.ExitCode { return codes } -func (b BatchReturn) CodeAt(n uint64) exitcode.ExitCode { +func (b BatchReturn) CodeAt(n uint64) (exitcode.ExitCode, error) { + if n >= uint64(b.Size()) { + return exitcode.Ok, errors.New("index out of bounds") + } for _, fc := range b.FailCodes { if fc.Idx == n { - return fc.Code + return fc.Code, nil + } + if fc.Idx > n { + return exitcode.Ok, nil + } + } + return exitcode.Ok, nil +} + +func (b BatchReturn) Validate() error { + size := uint64(b.Size()) + var gaps uint64 + for i, fc := range b.FailCodes { + if fc.Idx >= size { + // will also catch the case where the gaps aren't accounted for in total size + return errors.New("index out of bounds") + } + if i > 0 { + if fc.Idx <= b.FailCodes[i-1].Idx { + return errors.New("fail codes are not in strictly increasing order") + } + gaps += fc.Idx - b.FailCodes[i-1].Idx - 1 + } else { + gaps += fc.Idx } } - return exitcode.Ok + return nil } diff --git a/batch/batch_return_test.go b/batch/batch_return_test.go index 1f721eb8..92d8f6a3 100644 --- a/batch/batch_return_test.go +++ b/batch/batch_return_test.go @@ -12,25 +12,30 @@ import ( // Tests to match with Rust fil_actors_runtime::serialization func TestSerializationBatchReturn(t *testing.T) { testCases := []struct { + name string params BatchReturn hex string }{ { + name: "empty", params: BatchReturn{}, // [0,[]] hex: "820080", }, { + name: "single success", params: BatchReturn{SuccessCount: 1}, // [1,[]] hex: "820180", }, { + name: "single failure", params: BatchReturn{FailCodes: []FailCode{{Idx: 0, Code: exitcode.ErrIllegalArgument}}}, // [0,[[0,16]]] hex: "820081820010", }, { + name: "multiple success", params: BatchReturn{SuccessCount: 2, FailCodes: []FailCode{ {Idx: 1, Code: exitcode.SysErrOutOfGas}, {Idx: 2, Code: exitcode.ErrIllegalState}, @@ -42,15 +47,16 @@ func TestSerializationBatchReturn(t *testing.T) { } for _, tc := range testCases { - t.Run("", func(t *testing.T) { + t.Run(tc.name, func(t *testing.T) { req := require.New(t) var buf bytes.Buffer req.NoError(tc.params.MarshalCBOR(&buf)) req.Equal(tc.hex, hex.EncodeToString(buf.Bytes())) - var rt BatchReturn - req.NoError(rt.UnmarshalCBOR(&buf)) - req.Equal(tc.params, rt) + var br BatchReturn + req.NoError(br.UnmarshalCBOR(&buf)) + req.Equal(tc.params, br) + req.NoError(br.Validate()) }) } } @@ -58,48 +64,180 @@ func TestSerializationBatchReturn(t *testing.T) { func TestBatchReturn(t *testing.T) { req := require.New(t) - br := BatchReturn{} - req.Equal(0, br.Size()) - req.True(br.AllOk()) - req.Equal([]exitcode.ExitCode{}, br.Codes()) + t.Run("empty", func(t *testing.T) { + br := BatchReturn{} + req.Equal(0, br.Size()) + req.True(br.AllOk()) + req.Equal([]exitcode.ExitCode{}, br.Codes()) + _, err := br.CodeAt(0) + req.Error(err, "index out of bounds") + req.NoError(br.Validate()) + }) - br = BatchReturn{SuccessCount: 1} - req.Equal(1, br.Size()) - req.True(br.AllOk()) - req.Equal([]exitcode.ExitCode{exitcode.Ok}, br.Codes()) - req.Equal(exitcode.Ok, br.CodeAt(0)) + t.Run("single success", func(t *testing.T) { + br := BatchReturn{SuccessCount: 1} + req.Equal(1, br.Size()) + req.True(br.AllOk()) + req.Equal([]exitcode.ExitCode{exitcode.Ok}, br.Codes()) + ec, err := br.CodeAt(0) + req.NoError(err) + req.Equal(exitcode.Ok, ec) + _, err = br.CodeAt(1) + req.Error(err, "index out of bounds") + req.NoError(br.Validate()) + }) - br = BatchReturn{FailCodes: []FailCode{{Idx: 0, Code: exitcode.ErrIllegalArgument}}} - req.Equal(1, br.Size()) - req.False(br.AllOk()) - req.Equal([]exitcode.ExitCode{exitcode.ErrIllegalArgument}, br.Codes()) - req.Equal(exitcode.ErrIllegalArgument, br.CodeAt(0)) + t.Run("single failure", func(t *testing.T) { + br := BatchReturn{FailCodes: []FailCode{{Idx: 0, Code: exitcode.ErrIllegalArgument}}} + req.Equal(1, br.Size()) + req.False(br.AllOk()) + req.Equal([]exitcode.ExitCode{exitcode.ErrIllegalArgument}, br.Codes()) + ec, err := br.CodeAt(0) + req.NoError(err) + req.Equal(exitcode.ErrIllegalArgument, ec) + _, err = br.CodeAt(1) + req.Error(err, "index out of bounds") + req.NoError(br.Validate()) + }) - br = BatchReturn{SuccessCount: 1, FailCodes: []FailCode{{Idx: 1, Code: exitcode.ErrIllegalArgument}}} - req.Equal(2, br.Size()) - req.False(br.AllOk()) - req.Equal([]exitcode.ExitCode{exitcode.Ok, exitcode.ErrIllegalArgument}, br.Codes()) - req.Equal(exitcode.Ok, br.CodeAt(0)) - req.Equal(exitcode.ErrIllegalArgument, br.CodeAt(1)) + t.Run("multiple success", func(t *testing.T) { + br := BatchReturn{SuccessCount: 1, FailCodes: []FailCode{{Idx: 1, Code: exitcode.ErrIllegalArgument}}} + req.Equal(2, br.Size()) + req.False(br.AllOk()) + req.Equal([]exitcode.ExitCode{exitcode.Ok, exitcode.ErrIllegalArgument}, br.Codes()) + ec, err := br.CodeAt(0) + req.NoError(err) + req.Equal(exitcode.Ok, ec) + ec, err = br.CodeAt(1) + req.NoError(err) + req.Equal(exitcode.ErrIllegalArgument, ec) + req.Equal(exitcode.Ok, br.Codes()[0]) + _, err = br.CodeAt(2) + req.Error(err, "index out of bounds") + req.NoError(br.Validate()) + }) - br = BatchReturn{SuccessCount: 1, FailCodes: []FailCode{{Idx: 0, Code: exitcode.ErrForbidden}}} - req.Equal(2, br.Size()) - req.False(br.AllOk()) - req.Equal([]exitcode.ExitCode{exitcode.ErrForbidden, exitcode.Ok}, br.Codes()) - req.Equal(exitcode.ErrForbidden, br.CodeAt(0)) - req.Equal(exitcode.Ok, br.CodeAt(1)) + t.Run("multiple failure", func(t *testing.T) { + br := BatchReturn{SuccessCount: 1, FailCodes: []FailCode{{Idx: 0, Code: exitcode.ErrForbidden}}} + req.Equal(2, br.Size()) + req.False(br.AllOk()) + req.Equal([]exitcode.ExitCode{exitcode.ErrForbidden, exitcode.Ok}, br.Codes()) + ec, err := br.CodeAt(0) + req.NoError(err) + req.Equal(exitcode.ErrForbidden, ec) + ec, err = br.CodeAt(1) + req.NoError(err) + req.Equal(exitcode.Ok, ec) + _, err = br.CodeAt(2) + req.Error(err, "index out of bounds") + req.NoError(br.Validate()) + }) - br = BatchReturn{SuccessCount: 2, FailCodes: []FailCode{ - {Idx: 1, Code: exitcode.SysErrOutOfGas}, - {Idx: 2, Code: exitcode.ErrIllegalState}, - {Idx: 4, Code: exitcode.ErrIllegalArgument}, - }} - req.Equal(5, br.Size()) - req.False(br.AllOk()) - req.Equal([]exitcode.ExitCode{exitcode.Ok, exitcode.SysErrOutOfGas, exitcode.ErrIllegalState, exitcode.Ok, exitcode.ErrIllegalArgument}, br.Codes()) - req.Equal(exitcode.Ok, br.CodeAt(0)) - req.Equal(exitcode.SysErrOutOfGas, br.CodeAt(1)) - req.Equal(exitcode.ErrIllegalState, br.CodeAt(2)) - req.Equal(exitcode.Ok, br.CodeAt(3)) - req.Equal(exitcode.ErrIllegalArgument, br.CodeAt(4)) + t.Run("mixed", func(t *testing.T) { + br := BatchReturn{SuccessCount: 2, FailCodes: []FailCode{ + {Idx: 1, Code: exitcode.SysErrOutOfGas}, + {Idx: 2, Code: exitcode.ErrIllegalState}, + {Idx: 4, Code: exitcode.ErrIllegalArgument}, + }} + req.Equal(5, br.Size()) + req.False(br.AllOk()) + req.Equal([]exitcode.ExitCode{exitcode.Ok, exitcode.SysErrOutOfGas, exitcode.ErrIllegalState, exitcode.Ok, exitcode.ErrIllegalArgument}, br.Codes()) + ec, err := br.CodeAt(0) + req.NoError(err) + req.Equal(exitcode.Ok, ec) + ec, err = br.CodeAt(1) + req.NoError(err) + req.Equal(exitcode.SysErrOutOfGas, ec) + ec, err = br.CodeAt(2) + req.NoError(err) + req.Equal(exitcode.ErrIllegalState, ec) + ec, err = br.CodeAt(3) + req.NoError(err) + req.Equal(exitcode.Ok, ec) + ec, err = br.CodeAt(4) + req.NoError(err) + req.Equal(exitcode.ErrIllegalArgument, ec) + _, err = br.CodeAt(5) + req.Error(err, "index out of bounds") + req.NoError(br.Validate()) + }) +} + +func TestBatchReturn_Validate(t *testing.T) { + tests := []struct { + name string + batchReturn BatchReturn + errorMsg string + }{ + { + name: "valid batchreturn", + batchReturn: BatchReturn{ + SuccessCount: 5, + FailCodes: []FailCode{ + {Idx: 1, Code: exitcode.ErrIllegalArgument}, + {Idx: 3, Code: exitcode.ErrIllegalState}, + {Idx: 6, Code: exitcode.ErrNotPayable}, + }, + }, + }, + { + name: "failcodes not in strictly increasing order", + batchReturn: BatchReturn{ + SuccessCount: 5, + FailCodes: []FailCode{ + {Idx: 1, Code: exitcode.ErrIllegalArgument}, + {Idx: 3, Code: exitcode.ErrIllegalState}, + {Idx: 3, Code: exitcode.ErrNotPayable}, + }, + }, + errorMsg: "fail codes are not in strictly increasing order", + }, + { + name: "failcodes contain index out of bounds", + batchReturn: BatchReturn{ + SuccessCount: 5, + FailCodes: []FailCode{ + {Idx: 1, Code: exitcode.ErrIllegalArgument}, + {Idx: 3, Code: exitcode.ErrIllegalState}, + {Idx: 10, Code: exitcode.ErrNotPayable}, + }, + }, + errorMsg: "index out of bounds", + }, + { + name: "gaps between failures exceed successcount", + batchReturn: BatchReturn{ + SuccessCount: 2, + FailCodes: []FailCode{ + {Idx: 1, Code: exitcode.ErrIllegalArgument}, + {Idx: 4, Code: exitcode.ErrIllegalState}, + {Idx: 7, Code: exitcode.ErrNotPayable}, + }, + }, + errorMsg: "index out of bounds", + }, + { + name: "initial gap exceeds successcount", + batchReturn: BatchReturn{ + SuccessCount: 1, + FailCodes: []FailCode{ + {Idx: 2, Code: exitcode.ErrIllegalArgument}, + }, + }, + errorMsg: "index out of bounds", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + req := require.New(t) + err := tt.batchReturn.Validate() + // req.NoError(err) + if tt.errorMsg != "" { + req.ErrorContains(err, tt.errorMsg) + } else { + req.NoError(err) + } + }) + } }