-
Notifications
You must be signed in to change notification settings - Fork 118
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
tests: init pre-funded local authFactories and URIs usage #1814
base: main
Are you sure you want to change the base?
Conversation
testsLocalAllocations, err := tests.TestsRegistry.GenerateCustomAllocations(func() (chain.AuthFactory, error) { | ||
privateKey, err := ed25519.GeneratePrivateKey() | ||
if err != nil { | ||
return nil, err | ||
} | ||
return auth.NewED25519Factory(privateKey), nil | ||
}) |
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.
The lambda function could potentially be moved in auth
package.
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.
Makes sense
} | ||
return r.tests | ||
} | ||
|
||
func (r *Registry) GenerateCustomAllocations(generateAuthFactory func() (chain.AuthFactory, error)) ([]*genesis.CustomAllocation, error) { |
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.
The authFactories creation is delayed because when registering the tests, we don't know which private key type to use.
require.NoError(tn.ConfirmTxs(timeoutCtx, []*chain.Transaction{tx})) | ||
}) | ||
var _ = registry.Register(TestsRegistry, "Transfer Transaction", | ||
func(t ginkgo.FullGinkgoTInterface, tn tworkload.TestNetwork, authFactories ...chain.AuthFactory) { |
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 ...chain.AuthFactory
with []chain.AuthFactory
? I don't think we need the syntactic sugar here
func(t ginkgo.FullGinkgoTInterface, tn tworkload.TestNetwork, authFactories ...chain.AuthFactory) { | ||
require := require.New(t) | ||
ctx := context.Background() | ||
targetFactory := authFactories[0] |
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.
Should we add a require check that the length of authFactories
is 1 first?
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.
I will add a decorator function to check the authFactories length and fail fast, so the user can safely use the array
targetFactory := authFactories[0] | ||
authFactory := tn.Configuration().AuthFactories()[0] | ||
|
||
client := jsonrpc.NewJSONRPCClient(tn.URIs()[0]) | ||
balance, err := client.GetBalance(ctx, targetFactory.Address()) | ||
require.NoError(err) | ||
require.Equal(uint64(1000), balance) | ||
|
||
tx, err := tn.GenerateTx(ctx, []chain.Action{&actions.Transfer{ | ||
To: targetFactory.Address(), | ||
Value: 1, | ||
}}, |
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.
I think the flow should be authFactories
are assumed to be pre-funded with a specific amount (1000) in this case, and the test flow should be:
- confirm the provided balance is as expected
- generate and confirm transaction from the provided key (rather than the auth factory from the configuration)
- confirm balance updates after confirming the tx
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.
I have changed the test to have a pre-funded authFactory sending to another pre-funded authFactory 👍
I thought it was interesting to use both pre-funded authFactory
and config authFactory
. So the users see the two possibilities.
testsLocalAllocations, err := tests.TestsRegistry.GenerateCustomAllocations(func() (chain.AuthFactory, error) { | ||
privateKey, err := ed25519.GeneratePrivateKey() | ||
if err != nil { | ||
return nil, err | ||
} | ||
return auth.NewED25519Factory(privateKey), nil | ||
}) |
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.
This control flow is a bit confusing. This code is used to generate the genesis, but it's not clear from here where it's going to be used even though it's coupled to the tests registry.
We are using the test registry in the integration and e2e environments where we define two entry points. This is also where we invoke NewTestNetworkConfig
.
Could we change this function to take in a slice of genesis allocations as an argument (in addition to minBlockGap
)?
Then we can import the test registry and use the registered custom allocations in the entry point in both examples/morpheusvm/tests/integration/integration_test.go
and examples/morpheusvm/tests/e2e/e2e_test.go`
Relates to #1792
This PR is proposing a way to init pre-funded isolated authFactories.
It also adds an example of interacting with URIs to MorpheusVM transfer workload test.