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

[Morse->Shannon Migration] state export/import - collect accounts #1039

Open
wants to merge 9 commits into
base: scaffold/migration-module
Choose a base branch
from

Conversation

bryanchriswhite
Copy link
Contributor

@bryanchriswhite bryanchriswhite commented Jan 23, 2025

Summary

Add poktrolld migrate collect-morse-accounts command to convert Morse state export into a MorseAccountState for import into Shannon.

Issue

Type of change

Select one or more from the following:

Testing

  • Documentation: make docusaurus_start; only needed if you make doc changes
  • Unit Tests: make go_develop_and_test
  • LocalNet E2E Tests: make test_e2e
  • DevNet E2E Tests: Add the devnet-test-e2e label to the PR.

Sanity Checklist

  • I have tested my changes using the available tooling
  • I have commented my code
  • I have performed a self-review of my own code; both comments & source code
  • I create and reference any new tickets, if applicable
  • I have left TODOs throughout the codebase, if applicable

@bryanchriswhite bryanchriswhite self-assigned this Jan 23, 2025
@bryanchriswhite bryanchriswhite force-pushed the chore/migration/state-prep branch from f648aa5 to e35ca54 Compare January 24, 2025 15:19
@bryanchriswhite bryanchriswhite changed the base branch from scaffold/migration/morse-state to scaffold/migration-module January 27, 2025 11:31
@bryanchriswhite bryanchriswhite force-pushed the chore/migration/state-prep branch from 12a072c to 900c66b Compare January 27, 2025 11:32
Comment on lines 53 to 58
(gogoproto.jsontag) = "jailed",
(gogoproto.moretags) = "yaml:\"jailed\""];
int32 status = 4 [
(gogoproto.jsontag) = "status",
(gogoproto.moretags) = "yaml:\"status\""];
string staked_tokens = 6;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Reviewer do we need to consider these fields?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, but add the same qualifiers you have elsewhere. E.g. [(gogoproto.jsontag) = "tokens"];

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think my highlighting was off, I meant to call out jailed and status. If we do need these, what role should they play in the migration?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Regarding the json option, will add. 👍 It needs to be staked_tokens though as this structure is derived from the Morse export data structure (Morse proto types).

@bryanchriswhite bryanchriswhite force-pushed the chore/migration/state-prep branch 2 times, most recently from 3515058 to 20aa71d Compare January 27, 2025 11:55
// transformMorseState consolidates the Morse account balance, application stake,
// and supplier stake for each account as an entry in the resulting MorseAccountState.
//
// TODO_IN_THIS_COMMIT: benchmark and consider at what point (i.e. number of accounts) does asynchronous import processing matter?
Copy link
Contributor Author

@bryanchriswhite bryanchriswhite Jan 27, 2025

Choose a reason for hiding this comment

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

Seems to bottom out around 10^5 on my machine.

image

Copy link
Contributor Author

@bryanchriswhite bryanchriswhite Jan 27, 2025

Choose a reason for hiding this comment

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

Subsequent observations seem to indicate that performance actually seems to degrade with concurrency.

This comment was marked as resolved.

Copy link
Contributor Author

@bryanchriswhite bryanchriswhite Jan 28, 2025

Choose a reason for hiding this comment

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

I took a look at the number of accounts, apps, and suppliers on Morse mainnet; based on this information I don't think it's worthwhile to spend any effort on optimization/parallization here:

object count
accounts 56808
apps 2298
suppliers 13269
$ curl -X POST https://pocket-rpc.liquify.com/v1/query/accounts -d '{"page":1,"per_page":100000}' -o ./morse_accounts.json
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100 6671k    0 6671k  100    28  2788k     11  0:00:02  0:00:02 --:--:-- 2790k
 $ jq ".result|length" ./morse_accounts.json
56808
# curl -X POST https://pocket-rpc.liquify.com/v1/query/apps -d '{"opts": {"page":1,"per_page":100000}}' -o ./morse_apps.json
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100  600k    0  600k  100    38  1331k     84 --:--:-- --:--:-- --:--:-- 1333k
$ jq ".result|length" ./morse_apps.json
2298
$ curl -X POST https://pocket-rpc.liquify.com/v1/query/nodes -d '{"opts": {"page":1,"per_page":100000}}' -o ./morse_suppliers.json
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100 6172k    0 6172k  100    38  5039k     31  0:00:01  0:00:01 --:--:-- 5043k
$ jq ".result|length" ./morse_suppliers.json   
13269

@bryanchriswhite bryanchriswhite linked an issue Jan 27, 2025 that may be closed by this pull request
9 tasks
@bryanchriswhite bryanchriswhite force-pushed the chore/migration/state-prep branch from 08b72b7 to b19a003 Compare January 27, 2025 15:50
@bryanchriswhite bryanchriswhite force-pushed the chore/migration/state-prep branch from b19a003 to 890d5b5 Compare January 27, 2025 15:52
@bryanchriswhite bryanchriswhite marked this pull request as ready for review January 27, 2025 15:56
@Olshansk Olshansk added on-chain On-chain business logic consensus-breaking IMPORTANT! If the PR with this tag is merged, next release WILL HAVE TO BE an upgrade. labels Jan 28, 2025
@@ -0,0 +1,191 @@
package migrate
Copy link
Member

Choose a reason for hiding this comment

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

Given that this is all new code, let's use autocli instead.

Copy link
Contributor Author

@bryanchriswhite bryanchriswhite Jan 30, 2025

Choose a reason for hiding this comment

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

AutoCLI does not apply here because there is no gRPC service, message, or query.

The purpose of this command is to facilitate the deterministic (i.e. reproducible) transformation from the Morse export data structure (MorseStateExport) into the Shannon import data structure (MorseAccountState). It does not interact with the network directly.

proto/poktroll/migration/legacy.proto Outdated Show resolved Hide resolved
@@ -0,0 +1,42 @@
syntax = "proto3";
Copy link
Member

Choose a reason for hiding this comment

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

Can we rename this to morse.proto?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My rationale here was that we depend on multiple types from Morse that can be divided into two categories:

  1. Those which MUST be persisted on-chain in Shannon
  2. Those which are only required for the export/import state transform CLI command.

Given these categories, both of which I would characterize as "morse". I chose to go with "types.proto" for the on-chain types, for consistency with other modules, and then chose "legacy" for the other types.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Upon further reflection, I've realized that the ONLY types that are needed on-chain in Shannon are MorseAccountState, MorseAccount, and MorsePublicKey. I've moved MorseApplication and MorseValidator to legacy.proto reflect this.

proto/poktroll/migration/legacy.proto Outdated Show resolved Hide resolved
proto/poktroll/migration/legacy.proto Show resolved Hide resolved
cmd/poktrolld/cmd/migrate/migrate.go Show resolved Hide resolved
cmd/poktrolld/cmd/migrate/migrate.go Outdated Show resolved Hide resolved
cmd/poktrolld/cmd/migrate/migrate.go Show resolved Hide resolved
cmd/poktrolld/cmd/migrate/migrate.go Show resolved Hide resolved
cmd/poktrolld/cmd/migrate/migrate.go Outdated Show resolved Hide resolved
* scaffold/migration-module:
  chore: review feedback improvements
  refactor: migration keeper in cosmos app
  chore: fix some function names in comment (#1019)
@bryanchriswhite bryanchriswhite force-pushed the chore/migration/state-prep branch from 58fe5eb to 178ded8 Compare January 31, 2025 14:37
@Olshansk Olshansk added the migration Morse to Shannon migration related work label Jan 31, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
consensus-breaking IMPORTANT! If the PR with this tag is merged, next release WILL HAVE TO BE an upgrade. migration Morse to Shannon migration related work on-chain On-chain business logic
Projects
Status: 👀 In review
Development

Successfully merging this pull request may close these issues.

[Morse->Shannon Migration] Migration module
2 participants