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

feat!: native core SDK command support #892

Merged
merged 47 commits into from Jan 9, 2024
Merged

Conversation

Reecepbcups
Copy link
Member

@Reecepbcups Reecepbcups commented Dec 1, 2023

closes #885

ref: #256 may be a good time to start this after this PR merges

Builds all core transaction & query components from SDK v50

To backport to branch v7 for SDK v47, we have to redo some commands. I will manually backport in another PR

TODO

  • When tagging v8.1.0 I will go through this with all the changes, 99% are non state breaking for users
  • Gov queries should be the only state breaking change (will verify before tagging)

@Reecepbcups Reecepbcups added the dont_backport Do not backport to any other maintained branch label Dec 1, 2023
@Reecepbcups Reecepbcups removed the dont_backport Do not backport to any other maintained branch label Dec 3, 2023
chain/cosmos/authz.go Outdated Show resolved Hide resolved
@Reecepbcups Reecepbcups added the BACKPORT backport into all maintained branches label Dec 30, 2023
@@ -0,0 +1,204 @@
package cosmos
Copy link
Member Author

Choose a reason for hiding this comment

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

This file is unchanged, just moved from chain_node.go

Comment on lines +144 to +147
propId, err := strconv.ParseUint(proposalTx.ProposalID, 10, 64)
require.NoError(t, err, "failed to convert proposal ID to uint64")

_, err = cosmos.PollForProposalStatus(ctx, simdChain, height, height+heightDelta, propId, govv1beta1.StatusPassed)
Copy link
Member Author

Choose a reason for hiding this comment

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

minor formatting fix (use govv1beta1 type)

Comment on lines -13 to -28

// IBC-Go <= v7 / SDK <= v0.47
ProposalStatusUnspecified = "PROPOSAL_STATUS_UNSPECIFIED"
ProposalStatusPassed = "PROPOSAL_STATUS_PASSED"
ProposalStatusFailed = "PROPOSAL_STATUS_FAILED"
ProposalStatusRejected = "PROPOSAL_STATUS_REJECTED"
ProposalStatusVotingPeriod = "PROPOSAL_STATUS_VOTING_PERIOD"
ProposalStatusDepositPeriod = "PROPOSAL_STATUS_DEPOSIT_PERIOD"

// IBC-Go v8 / SDK v50
ProposalStatusUnspecifiedV8 = 0
ProposalStatusDepositPeriodV8 = 1
ProposalStatusVotingPeriodV8 = 2
ProposalStatusPassedV8 = 3
ProposalStatusRejectedV8 = 4
ProposalStatusFailedV8 = 5
Copy link
Member Author

Choose a reason for hiding this comment

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

Since we use the govv1beta1 or govv1 types now, these are not needed.

@@ -13,14 +13,14 @@ import (
// This token will be viewable by standard bank balance queries and send functionality.
// Depending on the chain parameters, this may require a lot of gas (Juno, Osmosis) if the DenomCreationGasConsume param is enabled.
// If not, the default implementation cost 10,000,000 micro tokens (utoken) of the chain's native token.
func TokenFactoryCreateDenom(c *CosmosChain, ctx context.Context, user ibc.Wallet, denomName string, gas uint64) (string, string, error) {
func (tn *ChainNode) TokenFactoryCreateDenom(ctx context.Context, user ibc.Wallet, denomName string, gas uint64) (string, string, error) {
Copy link
Member Author

Choose a reason for hiding this comment

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

Moves all to tn *ChainNode since we need to run Txs vs being on the first node

Comment on lines 190 to 192
func (c *CosmosChain) getFullNode() *ChainNode {
c.findTxMu.Lock()
defer c.findTxMu.Unlock()
if len(c.FullNodes) > 0 {
// use first full node
return c.FullNodes[0]
}
// use first validator
return c.Validators[0]
return c.GetNode()
}
Copy link
Member Author

Choose a reason for hiding this comment

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

When a test adds or removes a full node, this then switches which node the user was original acting on. This results in a NPE and causes a lot of confusion (including for myself). This uses GetNode() (validator[0]) now so commands always happen on the same machine as a user expects

Copy link
Member

Choose a reason for hiding this comment

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

great catch!!

Comment on lines +108 to +111
// GetCodec returns the codec for the chain.
func (c *CosmosChain) GetCodec() *codec.ProtoCodec {
return c.cdc
}
Copy link
Member Author

Choose a reason for hiding this comment

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

Required for module_auth.go (converting the any types -> accounts)

@@ -0,0 +1,210 @@
package cosmos
Copy link
Member Author

Choose a reason for hiding this comment

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

these functions were moved from chain_node.go. Only GovQuery* functions were added.

You can now specify to query govv1beta1 (< v47) or govv1 (SDK v50)

@@ -41,6 +25,14 @@ type TxProposalv1 struct {
Expedited bool `json:"expedited,omitempty"`
}

// ProtoMessage is implemented by generated protocol buffer messages.
// Pulled from github.com/cosmos/gogoproto/proto.
type ProtoMessage interface {
Copy link
Member Author

Choose a reason for hiding this comment

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

used in module_gov to not require SDK import (BuildProposal)

@Reecepbcups Reecepbcups changed the title feat: native core SDK command support feat!: native core SDK command support Dec 30, 2023
@Reecepbcups Reecepbcups removed the BACKPORT backport into all maintained branches label Dec 30, 2023
@Reecepbcups Reecepbcups marked this pull request as ready for review December 30, 2023 23:39
@Reecepbcups Reecepbcups requested a review from a team as a code owner December 30, 2023 23:39
Copy link
Member

@jtieri jtieri left a comment

Choose a reason for hiding this comment

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

this is a beast of a PR!! love the extension to include the core SDK commands as well as the reorganization of existing code.

had a few nits, nothing major at all. other than that this looks good to go 🚢 🚢 🚢

Comment on lines 190 to 192
func (c *CosmosChain) getFullNode() *ChainNode {
c.findTxMu.Lock()
defer c.findTxMu.Unlock()
if len(c.FullNodes) > 0 {
// use first full node
return c.FullNodes[0]
}
// use first validator
return c.Validators[0]
return c.GetNode()
}
Copy link
Member

Choose a reason for hiding this comment

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

great catch!!

chain/cosmos/module_auth.go Outdated Show resolved Hide resolved
chain/cosmos/module_authz.go Outdated Show resolved Hide resolved
chain/cosmos/module_authz.go Outdated Show resolved Hide resolved
chain/cosmos/module_feegrant.go Outdated Show resolved Hide resolved
@Reecepbcups Reecepbcups enabled auto-merge (squash) January 9, 2024 01:19
@Reecepbcups Reecepbcups merged commit 83d4ba2 into main Jan 9, 2024
11 checks passed
@Reecepbcups Reecepbcups deleted the reece/complete-core branch January 9, 2024 16:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add helpers for all Core SDK modules
2 participants