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] feat: implement MsgClaimMorseSupplier handler #1095

Open
wants to merge 245 commits into
base: main
Choose a base branch
from

Conversation

bryanchriswhite
Copy link
Contributor

@bryanchriswhite bryanchriswhite commented Feb 27, 2025

Summary

Implement MsgClaimMorseSupplier message handler.

Changes:

  • Implement MsgClaimMorseSupplier handler
  • Update MsgClaimMorseSupplier and MsgClaimMorseSupplierResponse types
  • Add EventMorseSupplierClaimed event
  • Add app-integration tests
  • Add module (msgServer/keeper) tests

Issue

Type of change

Select one or more from the following:

Sanity Checklist

  • I have updated the GitHub Issue assignees, reviewers, labels, project, iteration and milestone
  • For docs, I have run make docusaurus_start
  • For code, I have run make go_develop_and_test and make test_e2e
  • For code, I have added the devnet-test-e2e label to run E2E tests in CI
  • For configurations, I have update the documentation
  • I added TODOs where applicable

(cherry picked from commit 71f5112)
…nnon_dest_address morse_src_address morse_signature --response morse_src_address --response claimed_balance:coin --response claimed_at_height:int
bryanchriswhite and others added 7 commits March 7, 2025 16:35
…r' into issues/1034/scaffold/msg_claim_morse_supplier

* pokt/issues/1034/refactor/stake_supplier:
  fix: linter error
  chore: remove redundant field & update field indices
  [Code Health] fix: support zero kvcache TTL (#1108)
  chore: review feedback improvements
  [Morse->Shannon Migration] chore: add gateway, app & supplier keeper deps (#1079)
…ues/1034/feat/claim_morse_supplier

* issues/1034/scaffold/msg_claim_morse_supplier:
  fix: linter error
  chore: remove redundant field & update field indices
  [Code Health] fix: support zero kvcache TTL (#1108)
  chore: review feedback improvements
  [Morse->Shannon Migration] chore: add gateway, app & supplier keeper deps (#1079)
// The bech32-encoded address of the Shannon account to which will become the supplier operator.
// If empty, the shannon_owner_address will be used.
// See: https://dev.poktroll.com/operate/configs/supplier_staking_config#staking-types.
string shannon_operator_address = 2 [(cosmos_proto.scalar) = "cosmos.AddressString", (gogoproto.jsontag) = "shannon_operator_address"];

Copy link
Member

Choose a reason for hiding this comment

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

@bryanchriswhite Remember how I said that we'll have new requirements along the way (because it's hard to identify everything ahead of time).

tl;dr Not blocking this PR, but this comment is likely going to be the ref to a TODO_MAINNET/TODO_UPNEXT PR.


Here is another curveball:

If you run

$ pocket nodes stake

The definitions look like so (note the different output address for rewards/return of staked funds piece):

Stake the node into the network, making it available for service.

Usage:
  pocket nodes stake [command]

Available Commands:
  custodial     Stake a node in the network. Custodial stake uses the same address as operator/output for rewards/return of staked funds.
  non-custodial Stake a node in the network, non-custodial stake allows a different output address for rewards/return of staked funds. The signer may be the operator or the output address. The signer must specify the public key of the operator

If you run:

pocket query nodes --remoteCLIURL https://pocket-rpc.liquify.com >> /tmp/tmp.tx

Then this output is a list like so.

Note the address and output_address pieces.

        {
            "address": "01c6fff4e1596b5c9b5a6da1101ef2bc72ecfed4",
            "public_key": "1b92ad73f92df923e6eba9da8f0834ab02363f9ea859d66d82e0b104ac8281db",
            "jailed": false,
            "status": 2,
            "chains": [
                "F01B",
                "F016",
                "F00C",
                "F025",
                "F02E",
                "F017",
                "F032",
                "F01C"
            ],
            "service_url": "https://7690.n.poktstaking.com:443",
            "tokens": "60010000000",
            "unstaking_time": "0001-01-01T00:00:00Z",
            "output_address": "446cf0a9ad6d37b601439baf8329d163c36cbab9",
            "reward_delegators": {
                "cec3dd45e5b4a045ce9f88211ad5d2f066a81627": 35
            }
        },

As discussed previously (either explicitly or implicitly):

  1. We're ignoring jailed piece for now; if this changes, I'll let you know).
  2. We're ignoring reward_delegators; we simply respect/reflect the new settings only.
  3. We may or may not need to take unstaking_time into account. E.g. if you're unstaking during the snapshot, we treat you as staked on Shannon; I've CCed you on a discussion in notion
  4. We need to figure out output_address vs address (the whole custodial vs non-custodial piece)

The business logic I want us to add in the next PR is:

  1. Rename morse_src_address and morse_signature to morse_src_owner_address and morse_src_owner_signature respectively.
  2. If the output_address is empty in the morse state that's uploaded: apply the same business logic as you have in this PR (no change)
  3. If the output_address is non-empty: set the owner (output) & operator (non-output) address of the Supplier in Shannon accordingly.

This is a very high-level direction but lmk if it makes sense.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  1. We're ignoring jailed piece for now; if this changes, I'll let you know).
  2. We're ignoring reward_delegators; we simply respect/reflect the new settings only.
  3. We may or may not need to take unstaking_time into account. E.g. if you're unstaking during the snapshot, we treat you as staked on Shannon; I've CCed you on a discussion in notion

👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  1. Rename morse_src_address and morse_signature to morse_src_owner_address and morse_src_owner_signature respectively.
  2. If the output_address is empty in the morse state that's uploaded: apply the same business logic as you have in this PR (no change)
  3. If the output_address is non-empty: set the owner (output) & operator (non-output) address of the Supplier in Shannon accordingly.

👍

Copy link
Member

Choose a reason for hiding this comment

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

@bryanchriswhite Please create a GitHub Issue linking to this thread of comments as well as adding a TODO in the code linking to the github issue before merging!

bryanchriswhite and others added 10 commits March 11, 2025 08:40
…ake_app

* pokt/main:
  [Morse->Shannon Migration] scaffold `MsgClaimMorseApplication` message (#1080)
…laim_morse_app

* issues/1034/refactor/stake_app:
  [Morse->Shannon Migration] scaffold `MsgClaimMorseApplication` message (#1080)
  chore: review feedback improvements
  refactor: replace golang.org/x/exp with stdlib (#1112)
  [DOCS] Revamping the Full Node Documentation (#1096)
…morse_app

* pokt/main:
  [Morse->Shannon Migration] refactor: app staking logic as keeper methods (#1091)
…ake_supplier

* pokt/main:
  [Morse->Shannon Migration] implement `MsgClaimMorseApplication` handler (#1082)
…g_claim_morse_supplier

* pokt/main:
  [Morse->Shannon Migration] refactor: supplier `msgServer` methods to `Keeper` (#1093)
Copy link

The CI will now also run the e2e tests on devnet, which increases the time it takes to complete all CI checks.

You may need to run make trigger_ci to submit an empty commit that'll trigger the tests.

GCP workloads (requires changing the namespace to 1095)
Grafana network dashboard for devnet-issue-1095

@github-actions github-actions bot added devnet push-image CI related - pushes images to ghcr.io labels Mar 11, 2025
@bryanchriswhite bryanchriswhite changed the base branch from issues/1034/scaffold/msg_claim_morse_supplier to main March 11, 2025 13:12
…morse_supplier

* pokt/HEAD:
  [Morse->Shannon Migration] scaffold: `MsgClaimMorseSupplier` (#1094)

# Conflicts:
#	api/poktroll/migration/tx.pulsar.go
#	proto/poktroll/migration/tx.proto
#	tests/integration/migration/morse_claim_supplier_test.go
#	x/migration/keeper/msg_server_claim_morse_supplier.go
#	x/migration/keeper/msg_server_claim_morse_supplier_test.go
#	x/migration/simulation/claim_morse_supplier.go
#	x/migration/types/message_claim_morse_supplier.go
#	x/migration/types/message_claim_morse_supplier_test.go
#	x/migration/types/tx.pb.go
Copy link
Member

@Olshansk Olshansk left a comment

Choose a reason for hiding this comment

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

Pre-ematively approcing but PTAL at this comment: #1095 (comment)

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. devnet devnet-test-e2e migration Morse to Shannon migration related work on-chain On-chain business logic push-image CI related - pushes images to ghcr.io
Projects
Status: 👀 In review
Development

Successfully merging this pull request may close these issues.

[Morse->Shannon Migration] Migration module
2 participants