-
Notifications
You must be signed in to change notification settings - Fork 121
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
Add UTs for PreExecutor.PreExecute()
#1858
Changes from 10 commits
1595b77
f993239
b324ada
656d3ac
b48ca86
2fc73de
91e0f92
679a045
993ffc5
7dfcebd
d9d4d3e
de25e8c
2a934d8
553544b
c774d0d
e8900f1
686da59
811920c
ff73528
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,207 @@ | ||
// Copyright (C) 2024, Ava Labs, Inc. All rights reserved. | ||
// See the file LICENSE for licensing terms. | ||
|
||
package chain_test | ||
|
||
import ( | ||
"context" | ||
"errors" | ||
"testing" | ||
"time" | ||
|
||
"github.com/ava-labs/avalanchego/database" | ||
"github.com/ava-labs/avalanchego/database/memdb" | ||
"github.com/ava-labs/avalanchego/trace" | ||
"github.com/ava-labs/avalanchego/utils/set" | ||
"github.com/ava-labs/avalanchego/x/merkledb" | ||
"github.com/stretchr/testify/require" | ||
|
||
"github.com/ava-labs/hypersdk/chain" | ||
"github.com/ava-labs/hypersdk/genesis" | ||
"github.com/ava-labs/hypersdk/internal/validitywindow" | ||
"github.com/ava-labs/hypersdk/state" | ||
"github.com/ava-labs/hypersdk/state/metadata" | ||
"github.com/ava-labs/hypersdk/utils" | ||
) | ||
|
||
var ( | ||
feeKey = string(chain.FeeKey([]byte{2})) | ||
|
||
errMockAuth = errors.New("mock auth error") | ||
errMockValidityWindow = errors.New("mock validity window error") | ||
|
||
_ chain.Auth = (*mockAuth)(nil) | ||
_ chain.BalanceHandler = (*mockBalanceHandler)(nil) | ||
_ chain.ValidityWindow = (*mockValidityWindow)(nil) | ||
) | ||
|
||
type mockValidityWindow struct { | ||
isRepeatError error | ||
setBits []int | ||
} | ||
|
||
func (*mockValidityWindow) Accept(validitywindow.ExecutionBlock[*chain.Transaction]) { | ||
panic("unimplemented") | ||
} | ||
|
||
func (m *mockValidityWindow) IsRepeat(context.Context, validitywindow.ExecutionBlock[*chain.Transaction], []*chain.Transaction, int64) (set.Bits, error) { | ||
return set.NewBits(m.setBits...), m.isRepeatError | ||
} | ||
|
||
func (*mockValidityWindow) VerifyExpiryReplayProtection(context.Context, validitywindow.ExecutionBlock[*chain.Transaction], int64) error { | ||
panic("unimplemented") | ||
} | ||
|
||
func TestPreExecutor(t *testing.T) { | ||
testRules := genesis.NewDefaultRules() | ||
ruleFactory := genesis.ImmutableRuleFactory{ | ||
Rules: testRules, | ||
} | ||
|
||
tests := []struct { | ||
name string | ||
state map[string][]byte | ||
tx *chain.Transaction | ||
validityWindow chain.ValidityWindow | ||
verifyAuth bool | ||
err error | ||
}{ | ||
{ | ||
name: "valid test case", | ||
state: map[string][]byte{ | ||
feeKey: {}, | ||
}, | ||
tx: &chain.Transaction{ | ||
TransactionData: chain.TransactionData{ | ||
Base: &chain.Base{ | ||
Timestamp: utils.UnixRMilli( | ||
time.Now().UnixMilli(), | ||
testRules.GetValidityWindow(), | ||
), | ||
}, | ||
}, | ||
Auth: &mockAuth{ | ||
start: -1, | ||
end: -1, | ||
}, | ||
}, | ||
validityWindow: &mockValidityWindow{}, | ||
}, | ||
{ | ||
name: "raw fee doesn't exist", | ||
err: database.ErrNotFound, | ||
}, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we include a valid transaction for a test case like this where it will error before dereferencing the transaction? If the code were to change the order of verification, but the raw fee was still missing, ideally the test case would still pass. By writing the test this way, it makes it brittle and it will fail if we make any ordering changes to the code itself. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same comment for remaining test cases. To avoid duplicate code, we could construct a single valid tx above the creation of the slice of test cases. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we also add a more specific error, so that we won't confuse There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||
{ | ||
name: "validity window error", | ||
tx: &chain.Transaction{}, | ||
state: map[string][]byte{ | ||
feeKey: {}, | ||
}, | ||
validityWindow: &mockValidityWindow{ | ||
isRepeatError: errMockValidityWindow, | ||
}, | ||
err: errMockValidityWindow, | ||
}, | ||
{ | ||
name: "duplicate transaction", | ||
tx: &chain.Transaction{}, | ||
state: map[string][]byte{ | ||
feeKey: {}, | ||
}, | ||
validityWindow: &mockValidityWindow{ | ||
setBits: []int{0}, | ||
}, | ||
err: chain.ErrDuplicateTx, | ||
}, | ||
{ | ||
name: "tx state keys are invalid", | ||
state: map[string][]byte{ | ||
feeKey: {}, | ||
}, | ||
tx: &chain.Transaction{ | ||
TransactionData: chain.TransactionData{ | ||
Actions: []chain.Action{ | ||
&mockAction{ | ||
stateKeys: state.Keys{ | ||
"": state.None, | ||
}, | ||
}, | ||
}, | ||
}, | ||
Auth: &mockAuth{}, | ||
}, | ||
validityWindow: &mockValidityWindow{}, | ||
err: chain.ErrInvalidKeyValue, | ||
}, | ||
{ | ||
name: "verify auth error", | ||
state: map[string][]byte{ | ||
feeKey: {}, | ||
}, | ||
tx: &chain.Transaction{ | ||
TransactionData: chain.TransactionData{ | ||
Base: &chain.Base{}, | ||
}, | ||
Auth: &mockAuth{ | ||
verifyError: errMockAuth, | ||
}, | ||
}, | ||
validityWindow: &mockValidityWindow{}, | ||
verifyAuth: true, | ||
err: errMockAuth, | ||
}, | ||
{ | ||
name: "transaction pre-execute error", | ||
state: map[string][]byte{ | ||
feeKey: {}, | ||
}, | ||
tx: &chain.Transaction{ | ||
TransactionData: chain.TransactionData{ | ||
Base: &chain.Base{}, | ||
}, | ||
Auth: &mockAuth{}, | ||
}, | ||
validityWindow: &mockValidityWindow{}, | ||
err: chain.ErrTimestampTooLate, | ||
}, | ||
} | ||
|
||
for _, tt := range tests { | ||
t.Run(tt.name, func(t *testing.T) { | ||
r := require.New(t) | ||
ctx := context.Background() | ||
|
||
db, err := merkledb.New( | ||
ctx, | ||
memdb.New(), | ||
merkledb.Config{ | ||
BranchFactor: merkledb.BranchFactor16, | ||
Tracer: trace.Noop, | ||
}, | ||
) | ||
r.NoError(err) | ||
|
||
for k, v := range tt.state { | ||
r.NoError(db.Put([]byte(k), v)) | ||
} | ||
r.NoError(db.CommitToDB(ctx)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need to create a merkledb instance here? I think we can just the state type taken by the pre-executor to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Switched the |
||
|
||
preExecutor := chain.NewPreExecutor( | ||
&ruleFactory, | ||
tt.validityWindow, | ||
metadata.NewDefaultManager(), | ||
&mockBalanceHandler{}, | ||
) | ||
|
||
r.ErrorIs( | ||
preExecutor.PreExecute( | ||
ctx, | ||
nil, | ||
db, | ||
tt.tx, | ||
tt.verifyAuth, | ||
), tt.err, | ||
) | ||
}) | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we replace
setBits
with a value of typeset.Bits
? Not sure there's any advantage to handling this within the mockedIsRepeat
instead of leaving to the caller.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done