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.

Sign up for GitHub

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

Jump to bottom

[Gateway] Implement StakeGateway Message and Add Tests #68

Merged
merged 24 commits into from
Oct 18, 2023
Merged

[Gateway] Implement StakeGateway Message and Add Tests #68

merged 24 commits into from
Oct 18, 2023

Conversation

h5law
Copy link
Contributor

@h5law h5law commented Oct 15, 2023 •
edited
Loading

Summary

Human Summary

  • Add stake field to the stake and gateway proto messages
  • Update validation
  • Update and add unit tests relating to
    • genesis
    • cli
    • staking
  • Hydrate the stake keeper method with the correct logic
  • Add makefile helpers

Issue

Fixes (in part) 19

Type of change

Select one or more:

  • New feature, functionality or library
  • Bug fix
  • Code health or cleanup
  • Documentation
  • Other (specify)

Testing

  • Run all unit tests: make test_all_unit
  • Verify Localnet manually: See the instructions [here](TODO: add link to instructions)

Sanity Checklist

  • I have tested my changes using the available tooling
  • I have performed a self-review of my own code
  • I have commented my code, updated documentation and left TODOs throughout the codebase

@h5law h5law added gateway Changes related to the Gateway actor testing Test (or test utils) additions, fixes, improvements or other labels Oct 15, 2023
@h5law h5law added this to the Shannon TestNet milestone Oct 15, 2023
@h5law h5law self-assigned this Oct 15, 2023
@h5law h5law changed the base branch from main to scaffold_gateway_unstake October 15, 2023 14:21
@h5law h5law requested a review from red-0ne October 15, 2023 14:42
@h5law h5law force-pushed the hydrate_gateway_stake_message branch from 2490607 to ce085b0 Compare October 15, 2023 14:43
Base automatically changed from scaffold_gateway_unstake to main October 16, 2023 18:36
@h5law h5law force-pushed the hydrate_gateway_stake_message branch from ce085b0 to c91d50a Compare October 16, 2023 18:43
Olshansk
Olshansk requested changes Oct 17, 2023
View reviewed changes
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.

Overall LGTM, but I think the big piece is just catching up with the changes I made in the other PR.

.github/workflows/go.yml Outdated Show resolved Hide resolved
.gitignore Outdated Show resolved Hide resolved
proto/pocket/gateway/tx.proto Show resolved Hide resolved
x/gateway/client/cli/helpers_test.go Show resolved Hide resolved
x/gateway/client/cli/tx_stake_gateway.go Show resolved Hide resolved
x/gateway/client/cli/tx_stake_gateway.go Outdated Show resolved Hide resolved
x/gateway/client/cli/tx_stake_gateway_test.go Outdated Show resolved Hide resolved
x/gateway/keeper/msg_server_stake_gateway.go Outdated Show resolved Hide resolved
x/gateway/types/errors.go Outdated Show resolved Hide resolved
x/gateway/types/genesis.go Outdated Show resolved Hide resolved
@Olshansk
Copy link
Member

@h5law Also note that this is not "Hydration," but this is an implementation of the StakeGateway message. I recommend asking ChatGPT about the difference if you want to learn more.

Screenshot 2023-10-17 at 5 03 20 PM

@h5law h5law changed the title [Gateway] Hydrate StakeGateway Message and Add Tests [Gateway] Implement StakeGateway Message and Add Tests Oct 18, 2023
@h5law h5law requested a review from Olshansk October 18, 2023 16:32
@h5law
Copy link
Contributor Author

h5law commented Oct 18, 2023

@Olshansk have addressed the comments and caught up with #59 only thing I removed one of your helpers in the network test utils, I couldn't get it to work with the types. It only worked for the application genesis state and I couldn't figure out a good way to make it generic without stepping into encoding and decoding any types so thought best this be put into the helpers that call this method so simplicities sake.

Olshansk
Olshansk approved these changes Oct 18, 2023
View reviewed changes
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.

LGTM but PTAL at the two tiny comments before merging.

cmd/pocketd/cmd/root.go Outdated Show resolved Hide resolved
x/application/client/cli/helpers_test.go Show resolved Hide resolved
@h5law h5law merged commit 6b5f300 into main Oct 18, 2023
3 checks passed
@h5law h5law deleted the hydrate_gateway_stake_message branch October 18, 2023 17:01
bryanchriswhite added a commit that referenced this pull request Oct 19, 2023
@bryanchriswhite
Merge branch 'main' into feat/observable
ce3b055 
* main:
  [Miner, Supplier] chore: scaffold submit-proof message (#44)
  [Application] Implement MsgUnstakeApplication & Add Extensive Tests (#72)
  [Gateway] Implement UnstakeGateway message and add Tests (#75)
  [Gateway] Implement StakeGateway Message and Add Tests (#68)
  [Application] Implement MsgStakeApplication & Add Extensive Tests (#59)
  [CI] fix: run `make go_mockgen` step (#63)
bryanchriswhite added a commit that referenced this pull request Oct 19, 2023
@bryanchriswhite
Merge branch 'feat/observable' into merge/observable_x_supplier-msgs
69544fb 
* feat/observable:
  chore: last minute improvements
  chore: review improvements
  chore: improve comment
  chore: misc. review feedback improvements
  chore: improve comments
  refactor: rename `Observable#Close()` to `#UnsubscribeAll()`
  chore: improve comments
  chore: improve comments
  chore: cleanup, simplification, review improvements
  [Miner, Supplier] chore: scaffold submit-proof message (#44)
  [Application] Implement MsgUnstakeApplication & Add Extensive Tests (#72)
  [Gateway] Implement UnstakeGateway message and add Tests (#75)
  [Gateway] Implement StakeGateway Message and Add Tests (#68)
  [Application] Implement MsgStakeApplication & Add Extensive Tests (#59)
  [CI] fix: run `make go_mockgen` step (#63)
okdas added a commit that referenced this pull request Nov 14, 2024
@h5law @okdas
[Gateway] Implement StakeGateway Message and Add Tests (#68)
d7bfbeb 
Co-authored-by: DK <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Olshansk Olshansk Olshansk approved these changes

@bryanchriswhite bryanchriswhite Awaiting requested review from bryanchriswhite

@red-0ne red-0ne Awaiting requested review from red-0ne

Assignees

@h5law h5law

Labels
gateway Changes related to the Gateway actor testing Test (or test utils) additions, fixes, improvements or other
Projects
Shannon
Status: ✅ Done
Milestone
Shannon Private TestNet
Development

Successfully merging this pull request may close these issues.
3 participants
@h5law @Olshansk @okdas