Manual Testing for v0.43

[amaury1093]

This issue tracks all v0.43 changes we should be manually testing.

Existing Code Manual Testing

Each subteam should decide if each item should be assigned to 1 or 2 people, depending on importance and how big the item is.

West:

• Retest genesis migration after removal of power reduction @ryanchristo @AmauryM
  • 2 TESTs (both on local net, @AmauryM ):
    • 1. Create a software-upgrade proposal on old binary, wait for it to halt, run new binary. In-place store migrations all run.
    • 2. Create a dummy text proposal, vote on it, halt the old binary, run regen_old export (export on old binary), then regen migrate v0.43, then regen unsafe-reset-all, replace new genesis, then regen start. New chain starts correctly.
• #8077 Added support for grpc-web, enabling browsers to communicate with a chain's gRPC server
  • ensure grpc-web is enabled, test via browser gprcurl feature? - @aleem1314
• #8965 cosmos reflection now provides more information on the application such as: deliverable msgs, sdk.Config info etc (still in alpha stage).
  • ensure grpc reflection is working (via grpcurl) - @aleem1314
• #8559 Added Protobuf compatible secp256r1 ECDSA signatures. - @robert-zaremba
• #8786 Enabled secp256r1 in x/auth. - @robert-zaremba
• (rosetta) #8729 Data API fully supports balance tracking. Construction API can now construct any message supported by the application. - @robert-zaremba to coordinate w/ AiB - pending on Rosetta: more user guide and troubleshooting cosmos/cosmos-sdk#9575
• #8754 Added support for reverse iteration to pagination. - @ryanchristo
  • verify paginating some query commands in reverse works
  • TEST:
    • regen query staking validators
    • regen query staking validators --reverse
• (x/staking) #9214 Added new_shares attribute inside EventTypeDelegate event. - @technicallyty
  • Perform delegation, verify that event is correct
• #8363 Addresses no longer have a fixed 20-byte length. From the SDK modules' point of view, any 1-255 bytes-long byte array is a valid address. - @robert-zaremba
  • Create a >20-byte address (module address? new signing algo?) & verify that basic transfer & receive work as expected
• #8849 Upgrade module no longer supports time based upgrades. - @anilcse
  • Verify time based upgrades removed from CLI / REST / GRPC methods
• #8880 The CLI simd migrate v0.40 ... command has been renamed to simd migrate v0.42. - @aleem1314
  • verify that migration works (from server)
• #7477 Changed Bech32 Public Key serialization in the client facing functionality (CLI, MsgServer, QueryServer):- updated the keyring display structure (it uses protobuf JSON serialization) - the output is more verbose.- Renamed MarshalAny and UnmarshalAny to MarshalInterface and UnmarshalInterface respectively. These functions must take an interface as parameter (not a concrete type nor Any object). Underneath they use Any wrapping for correct protobuf serialization.- CLI: removed --text flag from show-node-id command; the text format for public keys is not used any more - instead we use ProtoJSON. - @robert-zaremba
  • Verify pubkey serialization on CLI can be used (e.g. for multisig key generation)
• fix: note flag description cosmos/cosmos-sdk#9436 #9134 Renamed the CLI flag --memo to --note. - @ryanchristo
  • verify all occurances of --memo field are replaced
• #9139 Querying events:- via ServiceMsg TypeURLs (e.g. message.action='/cosmos.bank.v1beta1.Msg/Send') does not work anymore,- via legacy msg.Type() (e.g. message.action='send') is being deprecated, new Msgs won't emit these events.- Please use concrete Msg TypeURLs instead (e.g. message.action='/cosmos.bank.v1beta1.MsgSend'). - @technicallyty
  • send events, verify message.action fields are correct
• #9291 Migration scripts prior to v0.38 have been removed from the CLI migrate command. The oldest supported migration is v0.39->v0.42. - @anilcse
  • verify migrate cmd's do not exit prior to v0.39
• #9540 Add output flag for query txs command. - @ryanchristo
  • TEST:
    cosmos-sdk@899b097da4b2f77e9929656b7025adb9d8f3a33b
regen q txs --help
regen tx bank send [address1] [address2] 100stake
regen q txs --events message.sender=[address1] --output json
• (x/staking) #9423 Staking delegations now returns empty list instead of rpc error when no records found. @aleem1314
  TEST:
  CLI:
  regen q staking delegations regen1st6788cgpqsjz2tkp86n7v5tlpz8ra834vmd8t
  gRPC-gateway:
  curl http:/127.0.0.1:1317/cosmos/staking/v1beta1/delegations/regen1st6788cgpqsjz2tkp86n7v5tlpz8ra834vmd8t
• #9408 fix: update simapp to use correct default broadcast mode - @ryanchristo
  • TEST:
    compare transaction modes:
    simd tx bank send [address1] [address2] 100stake
simd tx bank send [address1] [address2] 100stake --broadcast-mode block
• #9299 Fixed parse key issue @aleem1314
  • TEST:
    regen keys parse regen1drc2t8mckh7c8f72czznkwqjqy9zyqm4a9av8s
• #9534 Bring back deprecated proto fields to v1beta1 - @ryanchristo
  • Make sure creating a time-based upgrade plan throws an error
    • TEST:
      grpcurl -plaintext -d '{"tx_bytes":"encoded_tx","mode":"BROADCAST_MODE_SYNC"}' localhost:9090 cosmos.tx.v1beta1.Service/BroadcastTx
"rawLog": "time-based upgrades have been deprecated in the SDK: invalid request"
  • Make sure regen q gov votes returns Vote.Option != empty for legacy votes, and Vote.Option == empty for weighted votes.
    • TEST:
      option: VOTE_OPTION_UNSPECIFIED for legacy votes
      resolved and tested with fix: Correctly populate legacy Vote.Option field cosmos/cosmos-sdk#9583

East:

• (keyring) #8662 NewMnemonic now receives an additional passphrase argument to secure the key generated by the bip39 mnemonic. @cyberbono3
  • Skipped, as this is only an internal code change. Manual testing done on regen keys add --recover.
• (x/bank) #8798 GetTotalSupply is removed in favour of GetPaginatedTotalSupply
  • Querying total supply (via grpc) gives a paginated response @AmauryM
  • TEST:
    • grpcurl -plaintext -d '{"limit":2,"offset":2}' 157.245.115.184:9090 cosmos.bank.v1beta1.Query/TotalSupply // Returns no result
    • grpcurl -plaintext -d '{"limit":2,"offset":0}' 157.245.115.184:9090 cosmos.bank.v1beta1.Query/TotalSupply // Returns result
• Remove ServiceMsgs from ADR-031 cosmos/cosmos-sdk#9139 ServiceMsg TypeURLs (e.g. /cosmos.bank.v1beta1.Msg/Send) have been removed, as they don't comply to the Probobuf Any spec. Please use Msg type TypeURLs (e.g. /cosmos.bank.v1beta1.MsgSend). This has multiple consequences:
  • The sdk.ServiceMsg struct has been removed.
  • sdk.Msg now only contains ValidateBasic and GetSigners methods. The remaining methods GetSignBytes, Route and Type are moved to legacytx.LegacyMsg.
  • The RegisterCustomTypeURL function and the cosmos.base.v1beta1.ServiceMsg interface have been removed from the interface registry.
  • any CLI tx should test this?
    @blushi
• (x/gov) #7733 ADR 037 Implementation: Governance Split Votes
  • Test functionality of gov split votes via voting on txt proposal @likhita-809
• (x/staking) #8505 Convert staking power reduction into an on-chain parameter rather than a hardcoded in-code variable.
  • pending on Changing PowerReduction param causing network halt cosmos/cosmos-sdk#9447
• (x/bank) #8614 Add Name and Symbol fields to denom metadata
  • query denom metadata @cyberbono3
• (x/auth) #8522 Allow to query all stored accounts
  • query & paginate through all stored accounts @blushi
• (makefile) #7933 Use Docker to generate swagger files.
  • verify swagger generation runs as expected @AmauryM
  • TEST:
    • make proto-swagger-gen on the cosmos-sdk successfully passes
• (gRPC) #8945 gRPC reflection now works correctly.
  • test grpc reflection via grpcurl list & describe methods @cyberbono3
• (keyring) #8635 Remove hardcoded default passphrase value on NewMnemonic
  • Test out new mnemonic without passphrase @likhita-809
• (x/bank) #8434 Fix legacy REST API GET /bank/total and GET /bank/total/{denom} in swagger
  • test legacy REST API that both endpoints work as expected @likhita-809
• (x/slashing) #8427 Fix query signing infos command
  • verify regen query slashing singing-infos works as expected @AmauryM
• (x/upgrade) #8743 Add tracking module versions as per ADR-041
  • Successful upgrade of module version (in-place), must be tested w/ Cosmos Hub as regen already has the vesting account upgrade @ vitwit team @ryanchristo @clevinson
  • TEST:
    • https://hackmd.io/kStJjjb6RoO8WfTKhfOE0A
• #9429 Add cosmos_sdk_version to node_info @likhita-809
• #9513 Fixes testnet CLI command. Testnet now updates the supply in genesis. Previously, when using add-genesis-account and testnet together, inconsistent genesis files would be produced, as only add-genesis-account was updating the supply. @cyberbono3
• #9454 Fix testnet command with --node-dir-prefix accepts - and change node-dir-prefix token to testtoken. @likhita-809
• (x/gov) #8813 fix {appd} q gov deposits [proposal-id], GET /gov/proposals/{proposal_id}/deposits to include initial deposit. @atheeshp
• #9073 x/upgrade gRPC methods for VersionMap @AmauryM

Testing that might need code alterations

These tests are slightly more involved, and might need code alteration. Maybe 2 people can work on each item.

West:

• (x/bank) #9229 Now zero coin balances cannot be added to balances & supply stores. If any denom becomes zero corresponding key gets deleted from store. - @robert-zaremba
• #9133 Added hooks for governance actions. - @technicallyty @ryanchristo
  • Seems like an important feature, but don't think we can test purely with API testing
• #9514 Fix panic when retrieving the BlockGasMeter on (Re)CheckTx mode. @aleem1314
  • A fmt.Println of the headerHash on each block should do.

East:

• #9403 feat: add RefundGas function to GasMeter - @AmauryM
  • TEST:
    • send tx with normal node: consumed_gas = 50047
    • alter code and add ctx.GasMeter().RefundGas(10000, "") in antehandler, send tx: consumed_gas = 40047
    • alter code and add ctx.GasMeter().RefundGas(100000, "") in antehandler, send tx: node panics (and recovers)
• (crypto/ed25519) [#8690] Adopt zip1215 ed2559 verification rules. @AmauryM
  • Proposing to skip this one. I asked Marko, he doesn't know how to create ed25519 sigs that are NOT zip215-compliant.
  • Note: Tendermint is currently changing ed25519 library: crypto: Use a different library for ed25519/sr25519 tendermint/tendermint#6526. Might be able to test this on the new library.
• (store) #8012 Implementation of ADR-038 WriteListener and listen.KVStore @AmauryM
  • Nothing to test for v0.43, we should wait for part 2 (the streaming service)
• (optional) Test Tendermint v0.34.11 State Sync

authz & feegrant Manual Testing

These items test new modules' query & msg service. The items are divided by subteams, and constitute the minimum each subteam needs to test. Of course, the more testing there is the better.

West:

• regen tx authz grant: Granter can issue grants of normal & generic authorizations - @aleem1314
• regen tx authz revoke: Granter can revoke authorizations & grantee can no longer execute msg's - @aleem1314
• feegrant grant period not resetting cosmos/cosmos-sdk#9439 regen tx feegrant grant: Granter can issue fee grants of basic & periodic allowances, and grantee can spend from allowance - @ryanchristo
• regen q feegrant grant: Querying for a specific fee grant from a granter/grantee pair works as expected - @ryanchristo
• authz fix: collect all responses from authz/MsgExec #9538 @aleem1314

East:

• regen tx authz exec: Grantee can execute authorized msg's @atheeshp
• regen q authz grants: Querying of grants returns the correct response @likhita-809
• regen tx feegrant revoke: Granter can revoke fee grant allowance, and grantee can no longer spend from allowance @likhita-809
• regen q feegrant grants: Querying of all fee grants for a grantee works as expected @cyberbono3
• #9450 feegrant fix: feegrant grant period not resetting @blushi
• #9522 authz feat: make authz MsgExec emit events from all executed msgs @blushi

Note: thanks Cory for preparing all the groundwork.

[aaronc]

Blocked by migration issues in the testnet. @anilcse let us know when you have updates

[technicallyty]

#9133 Added hooks for governance actions. - @technicallyty @ryanchristo
Seems like an important feature, but don't think we can test purely with API testing

There are currently no hooks being used by the regen devnet. Ryan and I reviewed the hooks_test.go file and it seemed ok. Should there be something more involved for this check?

[amaury1093]

There are currently no hooks being used by the regen devnet. Ryan and I reviewed the hooks_test.go file and it seemed ok. Should there be something more involved for this check?

An idea could be to change code to add a hook that panics (e.g. on proposal submission), then submit a proposal via CLI, then make sure the node panics.

I would still manually test this (on top of reading the _test.go file), which is the whole point of this issue.

[robert-zaremba]

The secp256r1 items:

• #8559 Added Protobuf compatible secp256r1 ECDSA signatures. - @robert-zaremba
• #8786 Enabled secp256r1 in x/auth.

were tested when doing the keyring support branch for SDK (which is still on hold).

[amaury1093]

I just tested this morning migrations on my local machine, focusing on weighted votes migration. Both in-place migrations and JSON migrations work 🎉

[amaury1093]

Sorry everyone for delaying this issue again, but we did merge a lot of stuff in the SDK's master between the creation of this issue and today, and I think we should test them too.

I just did an audit of the commit log since May 17th, and added items equally between both teams. Let's try to assign them during standups

[fdymylja]

Regarding:

(rosetta) #8729 Data API fully supports balance tracking. Construction API can now construct any message supported by the application. - @clevinson to coordinate w/ AiB

There's a limitation in rosetta currently, for example, if a chain was to start from height 0 and upgrade at height 100, and this upgrade implies a breaking change in the protobuf schema, due to how codec works rosetta will be uncapable to decode old types.

This means that, when running rosetta-cli, rosetta will be able to sync from block 100 onward, and will fail to sync before block 100 as protobuf will be unable to decode messages.

We have already experienced this issue whilst attempting to sync regen's ledger:

2021/06/23 12:15:26 {"code":3,"message":"encode/decode error","description":"returned when there are errors encoding or decoding information to and from the node","retriable":true,"details":{"info":"errUnknownField \"*types.Plan\": {TagNum: 2, WireType:\"bytes\"}: tx parse error"}}: retrying fetch for block {"index":8} after 0.851031s (prior attempts: 3)

I assume this comes from TX decoder which blocks unknown protobuf fields.

Then we have another limitation, which is rather a lack of clarity in the rosetta API spec: whilst minting new denoms and moving them to an address, rosetta-cli reports an error which states the following:

Error: unable to process blocks: unable to process block: account balance response does not contain currency 

An issue was opened into rosetta's github: coinbase/mesh-cli#230 in order to understand how to approach this kind of use case (which is critical, especially now that chains make use of IBC).

But so far we have got no response from them.

[amaury1093]

I'm closing this issue, as I see the rosetta item in OP has been checked. Thanks everyone that took part of it! 🙌