Skip to content

Conversation

@pdyraga
Copy link
Member

@pdyraga pdyraga commented Jul 5, 2022

Refs #3006

This change is greatly inspired by the work of @nkuba in #3028 but it is just a subset of the original work with some changes introduced.

First, sortition.Chain handle interface is introduced. In opposite to v1 approach, where we had a global chain handle, the new proposed approach is to keep a chain handle per module. This should make the chain code less complicated and allow introducing custom behavior to functions, if needed be the logic of the given module.

Second, chain.Address type based on string is introduced. We need a chain-agnostic implementation of a chain address to use in interfaces. So far, it's been an array of bytes, as presented in Signing interface, but string seems easier to use in most cases.

Last but not least, a local implementation of sortition.Chain handle is introduced.

Also, a set of simple test utils for assertions is added. Most of the time, such simple assertions are all we need, and having them extracted to a single place lets us avoid the assertion error message formatting boilerplate in test files. Long-term, if needed, we may use some more sophisticated assertion library.

This change is greatly inspired by the work of @nkuba in
#3028 but it is just a subset of
the original work with some changes introduced.

First, `sortition.Chain` handle interface is introduced. In opposite to v1
approach, where we had a global chain handle, the new proposed approach is to
keep a chain handle per module. This should make the chain code less complicated
and allow introducing custom behavior to functions, if needed be the logic of
the given module.

Second, `chain.Address` type based on `string` is introduced. We need
a chain-agnostic implementation of a chain address to use in interfaces. So far,
it's been an array of bytes, as presented in `Signing` interface, but `string`
seems easier to use in most cases.

Last but not least, a local implementation of `sortition.Chain` handle is
introduced.

Also, a set of simple test utils for assertions is added. Most of the time, such
simple assertions are all we need, and having them extracted to a single place
lets us avoid the assertion error message formatting boilerplate in test files.
Long-term, if needed, we may use some more sophisticated assertion library.
@pdyraga pdyraga requested a review from a team July 5, 2022 10:18
pdyraga added 2 commits July 5, 2022 14:51
Instead of returning an error, we should return a boolean saying if the
staking provider registered operator address. This way, we can
distinguish this situation from infrastructure-level errors.
Moved the code to internal directory to clearly indicate nothing in
`local.go` should be in the interest of the code outside of `keep-core`.
This is just a mock for `sortition.Chain` interface that we'll use in
future `sortition.go` unit tests.
@pdyraga pdyraga mentioned this pull request Jul 5, 2022
@pdyraga pdyraga self-assigned this Jul 5, 2022
Comment on lines +11 to +15
OperatorToStakingProvider() (chain.Address, bool, error)
EligibleStake(stakingProvider chain.Address) (*big.Int, error)
IsPoolLocked() (bool, error)
IsOperatorInPool() (bool, error)
JoinSortitionPool() error
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Non-blocking for this PR but can we document those functions?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I am planning to do it in the next PR, when adding ethereum implementation.

@lukasz-zimnoch lukasz-zimnoch enabled auto-merge July 6, 2022 07:36
@lukasz-zimnoch lukasz-zimnoch merged commit 2792b0c into main Jul 6, 2022
@lukasz-zimnoch lukasz-zimnoch deleted the sortition-chain branch July 6, 2022 09:25
@pdyraga pdyraga added this to the v2.0.0-m1 milestone Jul 15, 2022
@pdyraga pdyraga modified the milestones: v2.0.0-m0, v2.0.0-m4 Sep 30, 2022
@pdyraga pdyraga removed their assignment Sep 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants