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
[Morse->Shannon Migration] state export/import - collect accounts #1039
base: main
Are you sure you want to change the base?
[Morse->Shannon Migration] state export/import - collect accounts #1039
Changes from all commits
1231e6f
5235822
6d9c3d9
b225150
03b76e1
85992e2
890d5b5
95e1edf
77bc6a5
ef4c985
08a1dd2
a63ea9b
0d8505d
46c87be
d4f4a80
178ded8
c2cca35
54fabd9
9bf52b2
aa2859b
c7d725a
794b768
5d473f0
165cf40
6ad5099
7b25a9a
9ddea8a
6f4e992
63c0790
efb6d49
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Check failure on line 29 in cmd/poktrolld/cmd/commands.go
Check failure on line 39 in cmd/poktrolld/cmd/migrate/migrate.go
Check failure on line 39 in cmd/poktrolld/cmd/migrate/migrate.go
Check failure on line 39 in cmd/poktrolld/cmd/migrate/migrate.go
Check failure on line 40 in cmd/poktrolld/cmd/migrate/migrate.go
Check failure on line 41 in cmd/poktrolld/cmd/migrate/migrate.go
Check failure on line 44 in cmd/poktrolld/cmd/migrate/migrate.go
Check failure on line 44 in cmd/poktrolld/cmd/migrate/migrate.go
Check failure on line 45 in cmd/poktrolld/cmd/migrate/migrate.go
Check failure on line 49 in cmd/poktrolld/cmd/migrate/migrate.go
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.
Very easy to follow, ty
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.
How about validator stake?
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.
My understanding is that "supplier stake" is identical to "validator stake" in Morse terms. I intentionally chose Shannon terms for all the comments for consistency.
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.
Why is this a module account?
I did a grep and found my way here: https://github.com/pokt-network/pocket-core/blob/35c05eb89ec30b50239b24730d0aeab0002cfde2/x/auth/types/codec.go#L16
![Screenshot 2025-01-29 at 4 58 28 PM](https://private-user-images.githubusercontent.com/1892194/407923357-c3691802-d62e-4101-b4c3-fb26a1e0412f.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3Mzg5NDQ1ODIsIm5iZiI6MTczODk0NDI4MiwicGF0aCI6Ii8xODkyMTk0LzQwNzkyMzM1Ny1jMzY5MTgwMi1kNjJlLTQxMDEtYjRjMy1mYjI2YTFlMDQxMmYucG5nP1gtQW16LUFsZ29yaXRobT1BV1M0LUhNQUMtU0hBMjU2JlgtQW16LUNyZWRlbnRpYWw9QUtJQVZDT0RZTFNBNTNQUUs0WkElMkYyMDI1MDIwNyUyRnVzLWVhc3QtMSUyRnMzJTJGYXdzNF9yZXF1ZXN0JlgtQW16LURhdGU9MjAyNTAyMDdUMTYwNDQyWiZYLUFtei1FeHBpcmVzPTMwMCZYLUFtei1TaWduYXR1cmU9NGIyNTc4MWVmYzBjOTRlYTQ5MWZiNGY4ODJhNTMzYzVhZjc5ODY0MzEyZGI5ZWUyZDkxMmY2MGQzM2Q1OWY4MyZYLUFtei1TaWduZWRIZWFkZXJzPWhvc3QifQ.Ee4-r0T1Ed-PcriBDOgVv7coiWk2W8WDvHjN2cD8NF8)
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.
The question isn't quite clear to me. The Morse data structure
BaseAccount
is used in Morse, but is seralized as a pb.Any type. This is the reason for theMorseAuthAccount
type, which includes thetype
field (to avoid having to deal with this additional and unnecessary complexity). Also note that the module account data structure is different, hence the use of pb.Any.With respect to the morse state export / account state import, my understanding is that we're only interested in externally owned accounts. Do you see a reason to migrate module accounts 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.
If you're asking how I determined that a type value of
posmint/Account
is indicative of an EOA (or the inverse), was by looking at my local ~/.pocket/config/genesis.json after doing./pocket accounts create
and./pocket accounts set-validator <acct. addr>
. You'll see that theposmint/Account
type is associated with the created account.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.
Not sure if they should be migrated but they might (@Olshansk ?) contain funds.
This makes me think we should validate the total supply returned by Morse vs. the supply we get by summing all accounts balances in the resulting
morseImportWorkspace
.Or, are we OK not having the same total supply after the migration?
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.
@bryanchriswhite - ACK. No followups.
@red-0ne - they shouldn't. In a followup PR, when we claim as a staked address, it'll be added in both places (i.e. like assets/liabilities on a balance sheet).
Can you add a TODO_MAINNET somewhere in this PR w/ the description of considering a validation against
poket query supply
.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.
I neglected to comment on this thread, mostly because I wanted to see what @Olshansk thought first.
My perspective, at this point, is that we're likely going to encounter (and have to resolve) several edge cases if we want a high-precision supply-based validation. My intuition is that this will not be as simple as it may initially seem; at the very least, I feel that there's a decent amount of uncertainty involved. My concern is that it becomes increasingly difficult to get a high-precision match as we track down ever smaller, and more illusive, and unexpected contributing sources to supply.
Is validation of supply useful if it is not high-precision? I.e., would we be willing to consider some error tolerance, or does that defeat the purpose?
I don't see a technical reason why this would be the case, but if seems to me that the intention behind this kind of validation is more about inspiring confidence in network participants. I feel like this confidence could easily become undermined if it comes with an error tolerance baked in. It seems to me like we end up shining a light on something that could be perceived as an issue but should not be, in an attempt to ease users.
What is the actual goal and are we able to (or already) accomplish(ing) it via other, simpler means?
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.
ShannonSuppliers == MorseNode (aka Morse Servicer)
ShannonValidator == MorseValidator (aka Morse Full Node)
Only top 1000 of staked validators are ACTUAL validators in Morse
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.
My understanding was that Morse supplier/node/servicer actors are all tendermint validators, and that tendermint uses sortition over the all validators to determine the active/voting set. I would have to look deeper into how Morse handles supplier staking to confirm/deny. According to tendermint v0.34 docs, the only ways for validators to be added are via genesis or an EndBlock message.
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.
Correct.
See
pocket nodes stake...
Top
1000
nodes by stake are validators.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.
It's not clear to me whether we're just trying to align on terminology or if there's a specific ask or clarification intended here. 🤔
I looked at
pocket nodes stakeNew
and I did not see the connection between the pokt node/validator and the tendermint validator.