Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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: add vesting account handling #232
feat: add vesting account handling #232
Changes from 16 commits
97698d5
9a5283e
8e43f3f
91176e8
cece5af
371b1a9
4e6ff33
ba35512
8758b7f
30f23c5
c1a3588
6354fab
d600690
e85ecf0
73719ad
0616f38
1f9f209
cc055ba
e0e4f64
6a53241
91bdf17
563f8ea
7a5d81a
31efda6
918ba27
6fcac22
1b31894
c006195
d655091
71eb9f2
600230c
39fc74d
9be3448
234f23b
4df7892
ace57db
2dcdeaf
decd364
4d7e1ec
83c661a
2d39fd4
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
As above, why are these re-declared since they are already available inside the SDK?
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.
Hi @RiccardoM , I added the types on the side because I couldn't really get the account type with Codec.. so I did with brute force to parse them into json. Do you know a way how I can parse/unpack the accounts?
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.
@huichiaotsou In order to understand how to unpack accounts, it's worth to understand how they are handled from Cosmos and why they are packed in a strange way. The best way to do this is though a series of considerations:
AccountI
, to represent a generic accountVestingAccount
to represent a generic vesting accountAny
type to serialize them.So, in order to read the details of a vesting account you need to do the following:
Any
into anAccountI
interface:accountI
is also implementing theVestingAccountI
interface:From there, you can use
vestingAccount
however you wish since it will be of typeVestingAccount
.Side note
Since they can be of various types (
ContinousVestingAccount
,DelayedVestingAccount
andPeriodicVestingAccount
), the best way to storeVestingAccount
instances is to simply serialize them as JSON and store them inside a table. However, this might make it really hard to get some useful insights that we might want to display on BigDipper (such as total vested/unvested tokens, next unvesting period, etc).For this reason, I would like to ask @ryuash and @calvinkei how they are going to use the vesting data in order to decide how to store them accordingly.
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.
By default Big Dipper 2.0 will not be using this data in favor of Forbole X.
Inside the Forbole X wallet details page I would assume it would show xx tokens are locked until xx (maybe a countdown type of animation)
Maybe @calvinkei can clarify
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.
@ryuash Wouldn't it be interesting for BigDipper to show a global amount of locked/unlocked tokens? Asking @kwunyeung also.
That might be interesting for users, no? To know when the next large unlock will happen or other interesting things
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 is the UI for showing vesting tokens on forbole x
So, it needs to support "total vesting tokens" and pagination. Probably filter & sort by date as well
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.
Hahs, if we do some feature like that then we definitely cannot throw a JSONB in to psql.
I would need to get all possible vesting periods in the genesis file and query by xx time.
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.
@huichiaotsou In order to preserve forward-compatibility with new features that might come up I would suggest then adding a better handling for different vesting account types. We can do so by doing this:
This way we can handling the storing of different types properly, with various periods etc
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.
@RiccardoM I'm also interested in showing vesting amount on BD but I do agree with @ryuash that it could be in favour of Forbole X at some point if BD not showing them by default. I think we can have this feature showing on X first. I also wanna see if there will be any feedback or request on getting this feature on BD.
Agree not storing the vesting periods in
JSONB
as the structure is very clear that each record has an array of coins and a vested date. It's not any arbitrary json content where we may search in certain keys inside the json.