Multisig wrapper command #111
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
RodrigoAD
force-pushed
the
multisig-wrapper-command
branch
2 times, most recently
from
e3e6932
to
928f406
Compare
reductionista
force-pushed
the
multisig-wrapper-command
branch
from
d7fe06b
to
101adfe
Compare
Closed
krebernisak
reviewed
Feb 17, 2022
packages-ts/gauntlet-terra-contracts/src/commands/abstract/inspectionWrapper.ts
Show resolved
Hide resolved
packages-ts/gauntlet-terra-contracts/networks/.env.bombay-testnet
Outdated
Show resolved
Hide resolved
RodrigoAD
force-pushed
the
multisig-wrapper-command
branch
from
101adfe
to
7365598
Compare
krebernisak
reviewed
Feb 17, 2022
Comment on lines
63
to
66
| if (!AccAddress.validate(process.env.MULTISIG_ADDRESS)) throw new Error(`Invalid Multisig wallet address`) | ||
| if (!AccAddress.validate(process.env.MULTISIG_GROUP)) throw new Error(`Invalid Multisig group address`) | ||
| this.multisig = process.env.MULTISIG_ADDRESS as AccAddress | ||
| this.multisigGroup = process.env.MULTISIG_GROUP as AccAddress |
There was a problem hiding this comment.
This should really be read from the input, and not directly from env vars here. Env vars should be completely abstracted from the command. The command just gets some kind of input (flags) that can be loaded from anywhere (CLI flags/args, env, file ...)
There was a problem hiding this comment.
Regarding loading environment variables, how about something like this:
const environment: Environment = load(.env filepath)
...
executeCLI(environment, myCommands)
Then, in each command:
this.multisig = this.env.MULTISIG_ADDRESS
where this.env contains the environment variables, available in each Command instance.
There was a problem hiding this comment.
Opening an issue here, thought could be a simpler fix, but Gauntlet is not really prepared to inject env vars anywhere.
#134
| // latest: { never: {} } | ||
| }, | ||
| } | ||
| return new MsgExecuteContract(signer, this.multisig, proposeInput) |
There was a problem hiding this comment.
Where do we define the wasm function to call? e.g. begin_proposal?
There was a problem hiding this comment.
Not sure is this is what you mean, but here we decide which operation to call: https://github.com/smartcontractkit/chainlink-terra/pull/111/files#diff-c7c2eafb6b93bd0e456c2fd133626df8051e82da80db2ad3695f45fe6cbd72bbR88
That's based on state.nextAction
packages-ts/gauntlet-terra-cw20-multisig/src/commands/multisig.ts
Outdated
Show resolved
Hide resolved
Comment on lines
187
to
198
| printPostInstructions = async (proposalId: number) => { | ||
| const state = await this.fetchState(proposalId) | ||
| const approvalsLeft = state.threshold - state.approvers.length | ||
| const messages = { | ||
| [Action.APPROVE]: `The proposal needs ${approvalsLeft} more approvals. Run the same command with the flag --proposal=${proposalId}`, | ||
| [Action.EXECUTE]: `The proposal reached the threshold and can be executed. Run the same command with the flag --proposal=${proposalId}`, | ||
| [Action.NONE]: `The proposal has been executed. No more actions needed`, | ||
| } | ||
| logger.line() | ||
| logger.info(`${messages[state.nextAction]}`) | ||
| logger.line() | ||
| } |
There was a problem hiding this comment.
This is something we used to call an inspection command.
Here it's just a function, on a different command class.
The Multisig command, that groups all Multisig abstract commands?
How do I call this inspection command? How do I compose it in a workflow?
Is the Multisig command a workflow or is it something else? What exactly?
I have many questions :)
...
The first priority is to have it function in any way possible, and while this looks ok IMO we need to go back to the Gauntlet v4 drawing board and start typing and composing these commands better.
There was a problem hiding this comment.
I see your point. This goes a bit beyond from this PR. Opening an issue for it: #133
Comment on lines
210
to
219
| logger.info(`Proposal State: | ||
| - Total Owners: ${state.owners.length} | ||
| - Owners List: ${state.owners} | ||
|
|
||
| - Threshold: ${state.threshold} | ||
| - Total Approvers: ${state.approvers.length} | ||
| - Approvers List: ${state.approvers} | ||
|
|
||
| - Next Action: ${state.nextAction.toUpperCase()} | ||
| `) |
There was a problem hiding this comment.
This is another inspection command baked in ad-hoc.
This was referenced Feb 18, 2022
raw tx on inspection command
list some todos
RodrigoAD
force-pushed
the
multisig-wrapper-command
branch
from
b7fc67d
to
45a7bd0
Compare
krebernisak
approved these changes
Feb 18, 2022
akhilchainani
added a commit
that referenced
this pull request
Mar 8, 2022
* bugfix: fix abstract command abi validation to work with cw-plus contracts These use anyOf instead of oneOf to list the available functions * add report output (#131) Also adds the option of setting what the report name will be via the environment variable "REPORT_NAME" which aids in automated tests. * Remove crates/query-proxy We no longer have the flags proxy so there's no sense to share definitions. * Multisig wrapper command (#111) * commands export data raw tx on inspection command * basic package and schema * multisig command * multisig command improvements list some todos * more detailed state * execute option on multisig * refactor * Feature/29149 multisig group commands (#132) * 29141: update_admin command * 29149: update_members command * 29149: export update_admin and update_memners commands * 29149: MR feedback - file reorganisation * 29149: applying formatting * pkg/terra: add Test_parseAttributes (#128) * fix tests (#139) * correctly set cosmos queries * rename juelsperluna to juelsperfeecoin in the set_config event * add check to make sure all fields are extracted from logs * switch to cropping the config digest before publishing to kafka. * add test for decoding configuration, transmission and balance from the chain read * revert to using the decoded config digest * use the relay monitoring Logger interface instead of the core logger * fix terra monitoring main * migration up test boiler (#138) * fix chaos CI (#140) * refactor ocr2:deploy --id to args[0] (#137) * changed flag on proxy command (#144) * fix proxy query for decimals, version, description * Improvements after Multisig testing (#145) * inspection improvements improvements on inspection * inspect msig comand * max voting period 7 days * Document lack of transmitters prefix (#150) * additional unit tests for remaining proxy functions (#149) * add ocr2 spec changes, switch image repo (#152) * Add gauntlet e2e tests through accepting a proposal (#141) * Add a hello-world consumer example (#143) * Add a hello-world consumer example * wrap up consumer contract + test cases * remove unneeded boilerplate - simplify mocked contract Co-authored-by: aalu1418 <aalu1418@gmail.com> * change env naming convention (#155) * proxy cmds uses args[0] and pull aggregator from RDD (#148) * add codeId for proxy_ocr2 testnet (#undefined) * feat: update OCR2 spec examples (#154) * Add Ocr2 Proxy E2E Test (#162) * implement the contracts for proxy so we can deploy them * Add ocr2 proxy test for latestRound, decimals, and description * pkg/terra: add MsgEnqueuer.GetMsgs (#113) * fix inspection bugs + improvements (#161) * proxy monitoring stub * move chain reader into its own file * proxy source factory uses the proxy address. Also update corresponding data generator for tests * add prometheus exporter factory and test, along with metrics and mocks * fix the issues reported by the linter * bump chainlink-relay to get the panic recover feature from Source * bump chainlink-relay dependency * add support for monitoring feeds that don't have a proxy contract configured * fix url path for fetching transactions from FCD * bump to the latest relay version where tx failed/succeeded metrics are counters instead of gauges * sequence requests to the terra rpc endpoint for all sources * bump chainlink-relay * introduce individual sequencers for TxsEvents and ContractStore * bump chainlink-relay and add GetType to all the terra monitoring source * fix go.sum * Token commands + Improvements (#157) * added balance diff in inspection * transfer token and send uluna commands * added mainnet ids * refactor before execution * refactor and improvements * fix rebase * Gauntlet sec improvements (#166) * transfer ownership checks * inpect offchain config from event info * provider in execution context. minor improvements * updated test * hex to base64 * Update guantlet e2e test to use an rdd with all non zero false values so the inspect command can check all values have changed No longer use the digest in the test as it is not needed Co-authored-by: Tate <tate.exon@smartcontract.com> * added cw20 code id and more validations * small refactor Co-authored-by: Tate <tate.exon@smartcontract.com> * unit test: reverted payments do not change owed balance (#169) * Payout oracles when set_billing is called (#168) * fix: rename accept_proposal method attribute * payout oracles when billing is set * Support negative observations (#165) * Use generic helper * Add a test * Placate linter * Address comments * Linter * 30252 tx simulation (#159) * 30252: TX Simulation method in AbstractCommand * 30252: Adding isSimulate instruction field to all oc2 initialize flow commands * 30252: Formatting * 30252: Removing optional isSimulate field (i.e always simulate) * 30252: Tx simulation for the arbitrary provided signer * 30252: batch simulation * 30252: tx simulation for multisig * 30252: Adjusting simulation success message * 30252: Formatting * Fee coin bounds check (#172) * Fee coin bounds check * Sanity check reimbursement calc * Cargo fmt * include chain_id length in config digest (#173) * include chain_id length in config digest * validate length below uint8 max, test case assert error * Providing raw proposal id for command input (#179) * Multisig simulation: generating tx using provider instead of wallet to avoid default assignments (#178) * drop the namespace from the logger * make the proxy source fetch and export the link available for payments of an aggregator contract * remove the link_available_for_payments metric on cleaup * ensure Client methods respect context cancel or expired * bump chainlink-relay to get the fix on the balances/observations precision * More mocked sources for tests (#156) * More mocked sources tests * in progress * Fix regex (#182) * Feature/31183 settings diff (#171) * added balance diff in inspection * transfer token and send uluna commands * added mainnet ids * refactor before execution * refactor and improvements * transfer ownership checks * inpect offchain config from event info * added balance diff in inspection * transfer token and send uluna commands * added mainnet ids * refactor before execution * refactor and improvements * fix rebase * transfer ownership checks * inpect offchain config from event info * provider in execution context. minor improvements * 31183: adding printDiff * 31183: printDiff function * 31183: getLatestOCRConfig function * 31183: proposeConfig * 31183: proposeOffchainConfig * 31183: acceptProposal * 31183: Small fixes based on the code review * 31183: randomSecret requested while creating the proposal to guarantee deterministic check * 31183: Cleanup * 31183: Resolving conflicts with token-commands * added cw20 code id and more validations * 31183: longsToNumber into a separate function * 31183: Updating inspect command with getLatestOCRConfig * refactor encryptions to accept secret * 31183: Adding payees to proposedConfig * Update e2e gauntlet test to grab the offchain proposal secret from the report and use it in the accept proposal * lint format * 31183: make longsInObjToNumbers pure and add deepCopy implementation to utils * 31183: Formatting * 31183: Review implementation - reorderings, renamings, comments Co-authored-by: RodrigoAD <15104916+RodrigoAD@users.noreply.github.com> Co-authored-by: Tate <tate.exon@smartcontract.com> * Add instructions for running the e2e tests to the readme (#174) * Add instructions for running the e2e tests to the readme * Move to docs and use readme link to it * bring solana and terra more in sync * Defaulting migrationContract to sender accAddress (#183) * add prod testnet environment (#184) * Add default rdd setting, and require for acceptOwnership Minor changes related to -rdd gauntlet flag (suggested during multisig review): 1. Will default to "../reference-data-directory/directory-terra-mainnet.json" if not passed explicitly 2. acceptOwnership & transferOwnership will throw Error instead of warn, if there is no rdd flag The first one should make it so the second one can never happen, but this is better behavior just in case. And it matches what most other commands already do. * Update default multisig-proposal expiration time to 24 hours * Expose payees on the set_config event * Add a note regarding validation execution * Fix event names in owned crate * Fix CI by removing a non-working 'defaultFlags.rdd' * copy all versions of cosmwasm to /lib * upgrade contract codeIds to v0.1.5 * update testnet-internal and add new lines to all codeIds files Co-authored-by: Domino Valdano <2644901+reductionista@users.noreply.github.com> Co-authored-by: Tate <tate.exon@smartcontract.com> Co-authored-by: Blaž Hrastnik <blaz@mxxn.io> Co-authored-by: Rodrigo Ariza <15104916+RodrigoAD@users.noreply.github.com> Co-authored-by: Sergei Drugalev <sergei.drugalev@smartcontract.com> Co-authored-by: Jordan Krage <jmank88@gmail.com> Co-authored-by: Sergey Kudasov <f4hrenh9it@gmail.com> Co-authored-by: alexandru topliceanu <alexandru.topliceanu@gmail.com> Co-authored-by: Frank Zhu <fr@nkzhu.com> Co-authored-by: aalu1418 <aalu1418@gmail.com> Co-authored-by: Connor Stein <connor.stein@mail.mcgill.ca> Co-authored-by: Aaron Lu <50029043+aalu1418@users.noreply.github.com> Co-authored-by: Rens Rooimans <github@rensrooimans.nl> Co-authored-by: Gheorghe Strimtu <studentcuza@gmail.com> Co-authored-by: Akhil Chainani <akhil.chainani@smartcontract.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.