-
Notifications
You must be signed in to change notification settings - Fork 28
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
feat: introducing hub, our new wallet management #826
Conversation
f2ca87f
to
0b5f9be
Compare
0b5f9be
to
59f19eb
Compare
59f19eb
to
9edfafe
Compare
f0b3f07
to
4f06666
Compare
9edfafe
to
d3f137c
Compare
507d10b
to
90e37bc
Compare
90e37bc
to
97222bc
Compare
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.
Great job. I just added some minor comments.
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.
LGTM, I've reviewed EVM and some part of core namespace.
1e5a4fd
to
7b17ee3
Compare
7b17ee3
to
5459333
Compare
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.
LGTM
Summary
Adding Hub into our
wallets-core
.We've experimented with hub in the original PR and I applied comments as well. In this PR, we will add Hub and its functionality first then in a separate PR we will have an adapter to let us use Hub and legacy system alongside each other.
I tried to have more test coverage and also code documentation in this PR in compare to original PR. I also cleaned up namespace and provider code to have more code quality. One notable change is I renamed
and
toand_then
and alsoor
toor_else
to communicate the intention better. Previously, We calledand, or, before, after
hook actions, but I made into two separate thing,and_then
andor_else
are (logical) Operators andafter
andbefore
are hooks.How did you test this change?
If you need documentation on the concepts, it's available on original PR, other than that I tired to document code using JSDOC as well.
You can review this PR using unit and integration tests that it has, and also I suggest to review in this order:
hub/namespaces
hub/providers
hub/hub.ts
hub/store
/builders
/utils
Blocked by
Related PRs
Checklist: