Closed malikankit closed 3 years ago
IF @naomijub 's health is better by Wednesday 28 Apr THEN Ruud and Julia can collaborate on this ELSE Ruud and Fynn can collaborate on this
@ruuda :Let's also discuss this in our catch up call on Wednesday.
I created a branch at 0e1c2edb77b593f1a41e190b93c3e13843113b3f
The code skeleton is at dao/program
I think it can be a very simple design: We can have mainly one function:
process_instruction
that takes as an argument an instruction. It fails if less than 50% of signatures from members are present in the transaction.
The owner of LIDO and stake_pool will be a derived address from <DAO_program_id, bump_seed> (we can change that in the future so users can define multiple derived addresses by passing the seeds as an argument).
Suppose we want to add a validator, everything is done in the UI level.
We craft an instruction add_validator(..)
and pass it to the process_instruction
together with validators signatures signed in the transaction.
Inside the process_instruction
all we do is verify if enough signatures are valid and execute the add_validator(..)
instruction on behalf of the derived address < DAO_program_id, bump_seed>
We discovered https://github.com/project-serum/multisig which appears to do exactly what we need. Some notes from playing around with it:
So far I haven’t been able to make the UI connect to my local testnet. I edited this line in multisig-ui
to point to the program address of my locally deployed multisig program, and added the upgrade authority as well. It prints
Error: Invalid account discriminator
In the developer console. Investigating further ...
@joncinque : ^ Any thoughts/opinion on using Serum Multisig for Lido Multisig Governance?
This is probably related to how anchor fetches on-chain information. Anchor is the framework used to develop the multisig program, and I believe it includes publishing the instruction / state data on-chain. So if you've only published the new program for yourself, you may be missing this extra information. I'll have to look into anchor a bit more in general, but if you have specific questions about it, you can ask Armani in the Serum discord, since I believe he led the development on anchor.
Until we release the governance program, this multisig will likely be the best option for Lido program governance.
With the Rust client, I was able to create a multisig account on my local testnet. Now for testing the other features!
I pushed this to the cli
branch for now (https://github.com/ChorusOne/multisig/commit/9a074314619d43380d6270184dccb32bccb32bfe).
A bit more detail about how the Serum Multisig works. I am going to refer to the Multisig program as Multisig (with a capital M), and to a group of owners who vote on transactions as a multisig (lowercase) account. The Multisig program can facilitate many multisig accounts, and each multisig account can execute many transactions.
Instruction
. To propose a transaction, yet another account is created, that holds the instruction, a boolean per owner that indicates whether that owner approved the transaction, and a boolean to prevent executing twice.approve
in the Multisig program for a given multisig account and transaction account.execute_transaction
in the Multisig program for a given multisig account and transaction, and it will invoke_signed
the instruction, signed by the program derived account.How we would use it:
bpf_loader_upgradable::upgrade
, and that is how the multisig can be used to update programs.set_upgrade_authority
instruction. That way the multisig could later set the upgrade authority for Lido to a more elaborate governance program.How to interact with the Multisig program:
~/.config/solana/id.json
:grimacing: I’m just working my way through the Multisig functions, the next things to add are approve
and execute_transaction
, and then I should be able to upgrade a program on my local testnet through Multisig. The work in progress is here: https://github.com/ChorusOne/multisig/pull/1 (beware, public repository).
I just read the entire thread @ruuda : This is great progress, given the short timeline! :)
It's best to start with what you proposed : upgrade a program on my local testnet through Multisig
.
Meanwhile - I will work on a couple of issues
These will focus on the governance related additions to be made to these programs. eg. functions to change fees params etc.
One question : Will set_upgrade_authority
require any changes / function additions in the Lido Program? Or this will be external to the Lido Program code (as long as its upgrade authority is set to our Multisig PDA on deployment?).
Will set_upgrade_authority require any changes / function additions in the Lido Program? Or this will be external to the Lido Program code (as long as its upgrade authority is set to our Multisig PDA on deployment?).
It will not require changes, everything is external to Lido. The initial deployer of Lido must set the upgrade authority to the multisig PDA.
set_upgrade_authority
from the BPFLoaderUpgradable
program.BPFLoaderUpgradable
enforces that the change is signed by the program’s current upgrade authority. (See this doc comment.)set_upgrade_authority
(which is also what solana program set-upgrade-authority
would do), but instead of executing it directly, we wrap it in a multisig transaction.set_upgrade_authority
gets called, signed by the multisig PDA.What can be done when there's a malicious owner that keeps proposing an instruction to stagger the multisig? How is the process of updating the owners (adding, removing)?
What can be done when there's a malicious owner that keeps proposing an instruction to stagger the multisig?
What does “stagger the multisig” mean?
How is the process of updating the owners (adding, removing)?
The Multisig program has a function set_owners
and change_threshold
for this. (Now that I think of it, this is a bad design, because you can only perform one instruction per multisig transaction, so if you want to require unanimous support, you can’t atomically add a signer — you have to add the new owner and approve, and and then change the threshold and approve. I’ll open an issue about this. Edit: https://github.com/project-serum/multisig/issues/4)
But I think that function is not even needed. What you could do instead is just create a new multisig with the updated owners, and then use the old multisig to change the upgrade authority of Solido to the new multisig. This is simpler, it has less moving parts. The only downside I can think of is if you use the multisig to manage many programs, you need to do this for all of them.
What does “stagger the multisig” mean?
If I'm a malicious member I can keep proposing some transaction ahead of everyone to "block" the contract, as it processes only 1 tx per time.
But I think that function is not even needed. What you could do instead is just create a new multisig with the updated owners, and then use the old multisig to change the upgrade authority of Solido to the new multisig. This is simpler, it has less moving parts. The only downside I can think of is if you use the multisig to manage many programs, you need to do this for all of them.
I think you can upgrade the multisig with another address for the owners, in that way you don't need to change any of the programs it controls.
If I'm a malicious member I can keep proposing some transaction ahead of everyone to "block" the contract, as it processes only 1 tx per time.
As discussed offline, this is not an issue, because many proposed transactions can exist simultaneously; the proposer opens a new account, and the transaction and its status are stored in there.
I now have the CLI working to the point where it can successfully upgrade a program whose upgrade authority is set to the multisig’s derived address.
As discussed with @enriquefynn, I’ll also add a command to the CLI to change the owners of the multisig. It would be possible to create a new account with the new owners instead, but because multisig accounts cannot be closed at this time, it might be good to stick to a single account.
It would also be nice to add an --output json
option to the multisig CLI, just like solana
has. That way I can write an automated test that goes through the entire flow of setting up a multisig and upgrading a program.
The CLI can upgrade a program now, and it can also change its owners/threshold. I am still working on adding an automated test that calls all commands in sequence.
If we want the multisig to also propose calls to Lido, that’s now easy to add — if we have a solana_sdk::Instruction
, then it can generate a transaction for that. For example: https://github.com/ChorusOne/multisig/blob/86af57759fe0b86f00ff386501fedf9796edc4c9/cli/src/main.rs#L444-L457
The multisig
program can now build and view two instructions: upgrade a program, and change the multisig. If we want to use it for managing Lido and the stake pool, it would become a specific utility for interacting with Lido, no longer a generic multisig program. I quickly discussed with @enriquefynn and we think this is the best way forward:
multisig
repository as a Git submodule in this repository, but point it at our fork. This makes it easy to keep it up to date, should it change upstream.solana::Instruction
s. Then we can add Lido-specific commands.I’ll wait for the current code to be reviewed, and then I’ll organize it like this. After that I can continue adding management options to the CLI. Possibly we could even merge the Multisig and Lido CLI programs, so multisig
becomes a subcommand of solido
.