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

Template: Add types/genesis_test.go #1419

Closed
lumtis opened this issue Jul 27, 2021 · 2 comments · Fixed by #1489
Closed

Template: Add types/genesis_test.go #1419

lumtis opened this issue Jul 27, 2021 · 2 comments · Fixed by #1489
Assignees
Labels
component:spn Related to or beneficial for SPN development type:request Feature request.

Comments

@lumtis
Copy link
Contributor

lumtis commented Jul 27, 2021

Is your feature request related to a problem? Please describe.
The TestGenesisState_Validate tests from types/genesis_test.go are extremely important when creating a blockchain because Validate is the method that prevents the blockchain starting from an inconsistent state.

Describe the solution you'd like
We should automatically generate a types/genesis_test.go that contains a template for those tests and facilitate the developer the implementation of them.
We can also add new checks when scaffolding types (check that no duplicated IDs can be provided)

@lumtis lumtis added the type:request Feature request. label Jul 27, 2021
@ilgooz
Copy link
Member

ilgooz commented Jul 28, 2021

For the Validate funcs, I saw some repetitive logic in spn like checking duplicated items in slices. I propose to create a validator helper pkg in tendermint/spm to make it easy to validate some common things.

For this spesific case with slices, we can implement something like:

// CheckDuplicated returns isDuplicated=false if all of the itr returns with the same type but with a different value.
// when there is a duplicated item, duplicatedItem is not nil and equals to the first found duplicated item.
// itr called for each item.
validation.CheckDuplicated(items interface{}, itr func(item interface{}) (value interface{})) (isDuplicated bool, duplicatedItem interface{})


validation.CheckDuplicated(coordinators, func(item interface{}) interface{} {
    return item.(Coordinator).Address
}) 

For max. ids:

// CheckOK returns isAllOK=true if all of the itr returns with true.
// when there is a non ok item, firstNonOK is not nil and equals to the first found non ok item.
validation.CheckOK(items interface{}, itr func(item interface{}) (ok bool)) (isAllOK bool, firstNonOK interface{})

validation.CheckOK(coordinators, func(item interface{}) (ok bool) {
    return item.(Coordinator).CoordinatorId >= count
}) 

@lumtis lumtis added the component:spn Related to or beneficial for SPN development label Aug 10, 2021
@lumtis lumtis self-assigned this Aug 16, 2021
@lumtis
Copy link
Contributor Author

lumtis commented Aug 16, 2021

That's nice to avoid duplicated generated code as much as possible. Although we must also keep in mind that we scaffold code that is eventually aimed to be modified and customized afterward

For example, in the case of spn genesis, we check that there is no duplicated chain ID with a map inside a loop, but then we reuse the same map to check that a request is always associated with an existing chain ID. Also, by having a loop for every lists of the genesis, we provide a template that allows the develop to easily add custom logic for each types in the genesis

Finally, if we need to factorize code, I would be more confortable with generating the helpers in the chain source, so the developer keeps full control over the scaffolded code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:spn Related to or beneficial for SPN development type:request Feature request.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants