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

tests: init pre-funded local authFactories and URIs usage #1814

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

najeal
Copy link
Contributor

@najeal najeal commented Nov 22, 2024

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.

Comment on lines 77 to 83
testsLocalAllocations, err := tests.TestsRegistry.GenerateCustomAllocations(func() (chain.AuthFactory, error) {
privateKey, err := ed25519.GeneratePrivateKey()
if err != nil {
return nil, err
}
return auth.NewED25519Factory(privateKey), nil
})
Copy link
Contributor Author

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.

Copy link
Collaborator

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) {
Copy link
Contributor Author

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.

@najeal najeal marked this pull request as ready for review November 22, 2024 17:58
require.NoError(tn.ConfirmTxs(timeoutCtx, []*chain.Transaction{tx}))
})
var _ = registry.Register(TestsRegistry, "Transfer Transaction",
func(t ginkgo.FullGinkgoTInterface, tn tworkload.TestNetwork, authFactories ...chain.AuthFactory) {
Copy link
Collaborator

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]
Copy link
Collaborator

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?

Copy link
Contributor Author

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

Comment on lines 30 to 41
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,
}},
Copy link
Collaborator

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:

  1. confirm the provided balance is as expected
  2. generate and confirm transaction from the provided key (rather than the auth factory from the configuration)
  3. confirm balance updates after confirming the tx

Copy link
Contributor Author

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.

Comment on lines 77 to 83
testsLocalAllocations, err := tests.TestsRegistry.GenerateCustomAllocations(func() (chain.AuthFactory, error) {
privateKey, err := ed25519.GeneratePrivateKey()
if err != nil {
return nil, err
}
return auth.NewED25519Factory(privateKey), nil
})
Copy link
Collaborator

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`

@najeal najeal requested a review from aaronbuchwald November 27, 2024 21:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants