-
Notifications
You must be signed in to change notification settings - Fork 44
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
Multisig batching #406
Closed
Closed
Multisig batching #406
Changes from 26 commits
Commits
Show all changes
46 commits
Select commit
Hold shift + click to select a range
eaa9d66
gauntlet: multisig: Allow batch approving transactions
archseer de66a46
Merge proposePayees into proposeConfig
archseer 350db25
Merge proposeOffchainConfig into proposeConfig
archseer e6c8c94
Merge in finalizeProposal
archseer e9e87b7
Merge in createProposal
archseer c4628b6
fix: if signing without payer, need to use partialSign
archseer a7b133c
nix: Fix include libudev in library path
archseer b3807b7
anchor_shell: Expose RPC ports on the host
archseer 0b39301
nix: Update flake dependencies
archseer 191ba75
gauntlet: Update typescript
archseer 69a59a3
gauntlet: More reliable tx submission and confirmation
archseer cefe93b
gauntlet: Simplify local env, preload programs with canonical program…
archseer 98377f5
Update flake, node and rust versions
archseer d1e831b
Use step identifiers, don't resolve inputs too early
archseer 21f1ddf
gauntlet: setup.dev.flow: Pass link address into subsequent steps
archseer e7fc822
Execute createProposal + SystemProgram.createAccount in same tx
archseer c916a85
Restructure proposeConfig simulation/execution
archseer 65e673a
createProposal must be signed by the proposal account too
archseer 7bd7bd8
wip
archseer 2d3b207
fix: Confirmation level only worked with `confirmed`
archseer 3a0fb85
proposeConfig steps unfortunately need to be fully sequential
archseer 4f00fd6
Create payee accounts before using them, change create_account to sup…
archseer 60b6fbe
yarn format (simulateTransaction)
archseer 2bee824
fix: need to pass in proposalId as part of input too
archseer 1e58980
fix: The flow needs to pass through correct secret to approveProposal
archseer 6c3b2fa
nix: Fully bump go (instead of just GOPATH)
archseer 8e06e41
Remove sendTx
archseer c7db9ef
Don't call sendAndConfirm directly
archseer cf2aefd
Stop manulaly calling simulateTx everywhere...
archseer 5f2dfeb
Update us to web3.js 1.64.0 and v0 transactions
archseer b4cf8de
Stop extracting transmissions address from raw tx
archseer 57126d2
minor cleanup
archseer 77815b1
Use confirmLevel of 'confirmed'
archseer 117e1df
Bump gauntlet-core with the fix
archseer e4abd19
Fix approveProposal, correctly passing in the secret
archseer e564886
Add missing await
archseer b47c491
proposeConfig: figure out what the remaining steps are if resuming
archseer b2aab8c
fix: randomSecret output was undefined
archseer 9b6ec84
Fix jest tests (https://github.com/LedgerHQ/ledger-live/issues/763)
archseer e6bdee3
gauntlet: store.setWriter doesn't need to load both programs
archseer 551c510
fix: multisig:approve needed to actually execute the tx
archseer 41659cb
solana/web3.js 1.67 is now more reliable with confirmations
archseer b9e022f
We already simulate the tx beforehand
archseer 783c334
Use legacy transactions with Ledger until supported upstream
archseer 036a247
fix: Ledger codepath needs to set blockhash & feePayer
archseer b9c1353
wip
archseer File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,5 @@ | ||
nodejs 14.19.1 | ||
rust 1.59.0 | ||
nodejs 18.6.0 | ||
rust 1.64.0 | ||
golang 1.19.3 | ||
golangci-lint 1.45.2 | ||
pulumi 3.40.1 | ||
|
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
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
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
45 changes: 45 additions & 0 deletions
45
gauntlet/packages/gauntlet-serum-multisig/src/commands/approve.ts
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,45 @@ | ||
import { SolanaCommand, TransactionResponse } from '@chainlink/gauntlet-solana' | ||
import { PublicKey } from '@solana/web3.js' | ||
import { Result } from '@chainlink/gauntlet-core' | ||
import { logger } from '@chainlink/gauntlet-core/dist/utils' | ||
import { CONTRACT_LIST, getContract } from '../lib/contracts' | ||
|
||
export default class Approve extends SolanaCommand { | ||
static id = 'serum_multisig:approve' | ||
static category = CONTRACT_LIST.MULTISIG | ||
|
||
static examples = ['yarn gauntlet serum_multisig:approve --network=local [IDS...]'] | ||
|
||
constructor(flags, args) { | ||
super(flags, args) | ||
} | ||
makeRawTransaction = async (signer: PublicKey) => { | ||
const multisigAddress = new PublicKey(process.env.MULTISIG_ADDRESS || '') | ||
const multisig = getContract(CONTRACT_LIST.MULTISIG) | ||
const address = multisig.programId.toString() | ||
const program = this.loadProgram(multisig.idl, address) | ||
|
||
logger.info(`Approving transactions: ${this.args}`) | ||
|
||
// map ids over this | ||
const ixs = await Promise.all( | ||
this.args.map((tx) => | ||
program.methods | ||
.approve() | ||
.accounts({ | ||
multisig: multisigAddress, | ||
transaction: new PublicKey(tx), | ||
owner: signer, | ||
}) | ||
.instruction(), | ||
), | ||
) | ||
|
||
return ixs | ||
} | ||
|
||
//execute not needed, this command cannot be ran outside of multisig | ||
archseer marked this conversation as resolved.
Show resolved
Hide resolved
|
||
execute = async () => { | ||
return {} as Result<TransactionResponse> | ||
} | ||
} |
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
17 changes: 10 additions & 7 deletions
17
gauntlet/packages/gauntlet-solana-contracts/networks/.env.local
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,10 +1,13 @@ | ||
NODE_URL=http://127.0.0.1:8899 | ||
PROGRAM_ID_OCR2=E3j24rx12SyVsG6quKuZPbQqZPkhAUCh8Uek4XrKYD2x | ||
PROGRAM_ID_ACCESS_CONTROLLER=2ckhep7Mvy1dExenBqpcdevhRu7CLuuctMcx7G9mWEvo | ||
PROGRAM_ID_STORE=9kRNTZmoZSiTBuXC62dzK9E7gC7huYgcmRRhYv3i4osC | ||
|
||
LINK=9TD23DYGTgUSVGWMjQVZ9cqLrvFRbzJ7MguUncfDLbSG | ||
export PRIVATE_KEY=[9,218,36,113,218,176,180,196,27,75,171,187,105,81,84,58,52,79,85,169,125,13,0,102,214,246,82,252,133,222,160,252,193,218,154,28,253,34,136,185,53,68,165,141,248,188,247,143,17,100,91,130,75,49,212,131,37,18,151,175,201,153,131,185] | ||
|
||
BILLING_ACCESS_CONTROLLER=2KeBZNtEhe9ws5n7czJxYyAYvTfzD6vMEYWHvACxMGWd | ||
REQUESTER_ACCESS_CONTROLLER=5UF9xKW9rjJJTzwSmvf6EKTkwTnp8RspVCXNtE4xX59d | ||
LOWERING_ACCESS_CONTROLLER=5UF9xKW9rjJJTzwSmvf6EKTkwTnp8RspVCXNtE4xX59d | ||
PROGRAM_ID_OCR2=cjg3oHmg9uuPsP8D6g29NWvhySJkdYdAo9D25PRbKXJ | ||
PROGRAM_ID_ACCESS_CONTROLLER=9xi644bRR8birboDGdTiwBq3C7VEeR7VuamRYYXCubUW | ||
PROGRAM_ID_STORE=HEvSKofvBgfaexv23kMabbYqxasxU3mQ4ibBMEmJWHny | ||
|
||
# LINK=9TD23DYGTgUSVGWMjQVZ9cqLrvFRbzJ7MguUncfDLbSG | ||
|
||
# BILLING_ACCESS_CONTROLLER=2KeBZNtEhe9ws5n7czJxYyAYvTfzD6vMEYWHvACxMGWd | ||
# REQUESTER_ACCESS_CONTROLLER=5UF9xKW9rjJJTzwSmvf6EKTkwTnp8RspVCXNtE4xX59d | ||
# LOWERING_ACCESS_CONTROLLER=5UF9xKW9rjJJTzwSmvf6EKTkwTnp8RspVCXNtE4xX59d |
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
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
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
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
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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When is this going to be used?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reopening this, the whole purpose of the multisig command is validation. Here the users will approve proposals they don't know what they do.
If approval batching is required, it should be built within the multisig wrapper, so Gauntlet can make proper validations and inform the users about the actions they'll do.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a quick workaround from an immediate need: without batching we need a ledger approval per feed per owner. This at least reduces it to a command (per 6-7 feeds) per owner.
It would be great to fix this in the multisig wrapper, but it's a larger job since we'd need to provide inputs for multiple invocations, verify them then pack that into a single tx. This is slightly tricky UX-wise too, since each command can take additional flags, for example
--secret
perocr2:accept_proposal:multisig
.I think the multisig wrapper needs a general re-work to stop being so stateful too. It's very easy to accidentally call the code without a proposal (or worse yet, the code is faulty and fetches no proposal), which ends up creating new proposals rather than approving.
We've already had that happen on starknet:
proposalId
was missing in the implementation but the command was running successfully (and tests passing), creating new proposals over and over again: smartcontractkit/chainlink-starknet@dacaae3#diff-8257597e02534a1f29c2fcb789a25491779bd7ac33359412616550f5c422d83cR208