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

tx submit-proposal breaking CLI change in cosmos-sdk 0.46 #8871

Closed
dckc opened this issue Feb 7, 2024 · 11 comments · Fixed by #8902
Closed

tx submit-proposal breaking CLI change in cosmos-sdk 0.46 #8871

dckc opened this issue Feb 7, 2024 · 11 comments · Fixed by #8902
Assignees

Comments

@dckc
Copy link
Member

dckc commented Feb 7, 2024

What is the Problem Being Solved?

cosmos-sdk makes breaking CLI changes such that our integration tests fail and our agd cli docs should be updated

For example, they renamed submit-proposal to submit-legacy-proposal and introduced a different implementation for submit-proposal
cosmos/cosmos-sdk@62d9790
agoric-labs/cosmos-sdk@v0.45.11-alpha.agoric.4...agoric-labs:cosmos-sdk:Agoric#diff-d69f01ab3522c6%5B%E2%80%A6%5D9ed7760c4575147d8c4f (e

Description of the Design / Test Plan

options include:
for example, a snapshot test of the output of agd tx gov submit-proposal swingset-core-eval.

cc @gibson042 @JimLarson

@dckc dckc added enhancement New feature or request test agoric-cosmos labels Feb 7, 2024
@dckc dckc changed the title test for changes to agd CLI help text how to manage breaking changes to agd CLI from cosmos-sdk? Feb 7, 2024
@dckc
Copy link
Member Author

dckc commented Feb 7, 2024

@ivanlei points out more downstream impact of the agd tx gov submit-proposal CLI breaking change:

  • dapps: dapp-offer-up uses it
  • instagoric

@mhofman suggests

  • a3p synthetic-chain may need to support old and new CLI style

@ivanlei
Copy link
Contributor

ivanlei commented Feb 7, 2024

@dckc dckc changed the title how to manage breaking changes to agd CLI from cosmos-sdk? tx submit-proposal breaking CLI change in cosmos-sdk 0.46 Feb 7, 2024
@JimLarson
Copy link
Contributor

I've got a change coming that implements a "hybrid" submit-proposal that works either with the new or legacy syntax, and the hybrid is also aliased as submit-legacy-proposal. So we can use it with the current (legacy) invocations (yay!), and then over time all the call sites can either explicitly use submit-legacy-proposal, or upgrade to the new syntax. When all call sites have done one or the other of these choices, we can drop our modification to this command.

@JimLarson
Copy link
Contributor

The slight limitation of this approach is that you cannot name the new-syntax JSON file the same as one of the legacy subcommands:

  • cancel-software-upgrade
  • community-pool-spend
  • ibc-upgrade
  • param-change
  • software-upgrade
  • swingset-core-eval
  • update-client

JimLarson added a commit to agoric-labs/cosmos-sdk that referenced this issue Feb 8, 2024
Have submit-legacy-proposal be an alias for submit-proposal.
Invocations of the legacy syntax should either upgrade to the
new syntax, or explicitly use submit-legacy-proposal. When
all legacy usages have so migrated, this change can be dropped.
See Agoric/agoric-sdk#8871.
@JimLarson
Copy link
Contributor

CLI-breaking change was laid down in cosmos/cosmos-sdk#11013

@JimLarson
Copy link
Contributor

Manual testing results:

Random testing address created and set to shell var $boot.

proposal.json

{
  "messages": [
    {
      "@type": "/cosmos.bank.v1beta1.MsgSend",
      "from_address": "agoric1d3hfdh59jf43azxlcwm557vdtsf5pgnnw9kwny",
      "to_address": "agoric1d3hfdh59jf43azxlcwm557vdtsf5pgnnw9kwny",
      "amount":[{"denom": "ubld","amount": "10"}]
    }
  ],
  "deposit": "10ubld"
}

New API:

$ agd --chain-id agoriclocal tx gov submit-proposal --generate-only proposal.json --from $boot
{"body":{"messages":[{"@type":"/cosmos.gov.v1.MsgSubmitProposal","messages":[{"@type":"/cosmos.bank.v1beta1.MsgSend","from_address":"agoric1d3hfdh59jf43azxlcwm557vdtsf5pgnnw9kwny","to_address":"agoric1d3hfdh59jf43azxlcwm557vdtsf5pgnnw9kwny","amount":[{"denom":"ubld","amount":"10"}]}],"initial_deposit":[{"denom":"ubld","amount":"10"}],"proposer":"agoric1d3hfdh59jf43azxlcwm557vdtsf5pgnnw9kwny","metadata":""}],"memo":"","timeout_height":"0","extension_options":[],"non_critical_extension_options":[]},"auth_info":{"signer_infos":[],"fee":{"amount":[],"gas_limit":"200000","payer":"","granter":""},"tip":null},"signatures":[]}

proposal-legacy.json

{
  "title": "Test Proposal",
  "description": "My awesome proposal",
  "type": "Text",
  "deposit": "10ubld"
}

Legacy - proposal file:

$ agd --chain-id agoriclocal tx --generate-only=true gov --from $boot submit-proposal --proposal proposal-legacy.json
{"body":{"messages":[{"@type":"/cosmos.gov.v1beta1.MsgSubmitProposal","content":{"@type":"/cosmos.gov.v1beta1.TextProposal","title":"Test Proposal","description":"My awesome proposal"},"initial_deposit":[{"denom":"ubld","amount":"10"}],"proposer":"agoric1d3hfdh59jf43azxlcwm557vdtsf5pgnnw9kwny"}],"memo":"","timeout_height":"0","extension_options":[],"non_critical_extension_options":[]},"auth_info":{"signer_infos":[],"fee":{"amount":[],"gas_limit":"200000","payer":"","granter":""},"tip":null},"signatures":[]}

Legacy - proposal args

$ agd --chain-id agoriclocal tx --generate-only=true gov --from $boot submit-proposal --type Text --title "Test Proposal" --description "My awesome proposal" --deposit 10ubld
{"body":{"messages":[{"@type":"/cosmos.gov.v1beta1.MsgSubmitProposal","content":{"@type":"/cosmos.gov.v1beta1.TextProposal","title":"Test Proposal","description":"My awesome proposal"},"initial_deposit":[{"denom":"ubld","amount":"10"}],"proposer":"agoric1d3hfdh59jf43azxlcwm557vdtsf5pgnnw9kwny"}],"memo":"","timeout_height":"0","extension_options":[],"non_critical_extension_options":[]},"auth_info":{"signer_infos":[],"fee":{"amount":[],"gas_limit":"200000","payer":"","granter":""},"tip":null},"signatures":[]}

Legacy - subcommand

$ agd --chain-id agoriclocal tx --generate-only=true gov --from $boot submit-proposal cancel-software-upgrade --title foo --description bar
{"body":{"messages":[{"@type":"/cosmos.gov.v1beta1.MsgSubmitProposal","content":{"@type":"/cosmos.upgrade.v1beta1.CancelSoftwareUpgradeProposal","title":"foo","description":"bar"},"initial_deposit":[],"proposer":"agoric1d3hfdh59jf43azxlcwm557vdtsf5pgnnw9kwny"}],"memo":"","timeout_height":"0","extension_options":[],"non_critical_extension_options":[]},"auth_info":{"signer_infos":[],"fee":{"amount":[],"gas_limit":"200000","payer":"","granter":""},"tip":null},"signatures":[]}

@JimLarson
Copy link
Contributor

And in particular, the swingset-core-eval subcommand with permit.json and code.js set to just {}.

$ agd --chain-id agoriclocal tx --generate-only=true gov --from $boot submit-proposal swingset-core-eval --title foo --description bar permit.json code.js
{"body":{"messages":[{"@type":"/cosmos.gov.v1beta1.MsgSubmitProposal","content":{"@type":"/agoric.swingset.CoreEvalProposal","title":"foo","description":"bar","evals":[{"json_permits":"{}\n","js_code":"{}\n"}]},"initial_deposit":[],"proposer":"agoric1d3hfdh59jf43azxlcwm557vdtsf5pgnnw9kwny"}],"memo":"","timeout_height":"0","extension_options":[],"non_critical_extension_options":[]},"auth_info":{"signer_infos":[],"fee":{"amount":[],"gas_limit":"200000","payer":"","granter":""},"tip":null},"signatures":[]}

@JimLarson
Copy link
Contributor

Validation on emerynet:

echo "{}" > permit.json
echo "{}" > code.js
agd --home . tx gov submit-proposal swingset-core-eval --from a0 --title foo --description bar permit.json code.js
agd --home . query gov proposals

Can see the proposal in the query output:

- deposit_end_time: "2024-03-05T01:38:08.617787206Z"
  final_tally_result:
    abstain_count: "0"
    no_count: "0"
    no_with_veto_count: "0"
    yes_count: "0"
  id: "33"
  messages:
  - '@type': /cosmos.gov.v1.MsgExecLegacyContent
    authority: <redacted>
    content:
      '@type': /agoric.swingset.CoreEvalProposal
      description: bar
      evals:
      - js_code: |
          {}
        json_permits: |
          {}
      title: foo
  metadata: ""
  status: PROPOSAL_STATUS_DEPOSIT_PERIOD
  submit_time: "2024-03-03T01:38:08.617787206Z"
  total_deposit: []
  voting_end_time: null
  voting_start_time: null

@mhofman
Copy link
Member

mhofman commented Mar 7, 2024

Re-opening as there seem to be issues with chain software upgrade proposals

#1995 [use-upgrade-14-rc1 linux/amd64 prepare-upgrade-14-rc1 4/4] RUN ./run_prepare.sh b:upgrade-14-rc1
#1995 13.55 2024-03-07T00:16:16.645Z launch-chain: Launched SwingSet kernel
#1995 13.55 2024-03-07T00:16:16.647Z block-manager: block 1001 begin
#1995 13.56 2024-03-07T00:16:16.656Z block-manager: block 1001 commit
#1995 13.60 2024-03-07T00:16:16.701Z block-manager: block 1002 begin
#1995 13.61 2024-03-07T00:16:16.706Z block-manager: block 1002 commit
#1995 13.83 block produced
#1995 14.64 2024-03-07T00:16:17.734Z block-manager: block 1003 begin
#1995 14.64 2024-03-07T00:16:17.740Z block-manager: block 1003 commit
#1995 14.99 block produced
#1995 14.99 done
#1995 14.99 startAgd() done
#1995 14.99 [b:upgrade-14-rc1] Agd started.
#1995 14.99 [b:upgrade-14-rc1] Voting in the upgrade.
#1995 15.04 {}
#1995 15.04 upgrade-info: {}
#1995 15.07 Error: missing checksum query parameter
#1995 15.07 Usage:
#1995 15.07   agd tx gov submit-proposal software-upgrade [name] (--upgrade-height [height]) (--upgrade-info [info]) [flags]

@mhofman mhofman reopened this Mar 7, 2024
@mhofman
Copy link
Member

mhofman commented Mar 8, 2024

Before we close this issue, we need an a3p test for software upgrades. The problem is that to simulate the upgrade flow, we need a version to upgrade to. Either we set a version that doesn't exist and just verify that the upgrade passed (software halted), or we upgrade to another upgrade plan name supported by the same software, but I'm not sure our upgrade logic currently handles that (depending on if the cosmos upgrade module forces a halt and restart or not)

@mhofman
Copy link
Member

mhofman commented Mar 18, 2024

Closing in favor of #8884

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants