feat: contract is now upgradeable through participants voting for updates #714
Conversation
| &mut self, | ||
| code: Option<Vec<u8>>, | ||
| config: Option<Config>, | ||
| ) -> Result<UpdateId, VoteError> { |
There was a problem hiding this comment.
I was asking about this pattern in one of my previous PRs. Does it make sense to return more specific errors on all, or all user-facing functions instead of the generic MPC error?
There was a problem hiding this comment.
yeah we should flatten the errors out similar to workspace errors because we have too many similar sounding errors that can pop up for categories of errors. Let me see if I can whip up something real quick for this before release
| contract.view("state").await.unwrap().json().unwrap(); | ||
|
|
||
| // Let's propose a contract update instead now. | ||
| let new_wasm = std::fs::read(CONTRACT_FILE_PATH).unwrap(); |
There was a problem hiding this comment.
Technically, it is the same contract.
Not blocking, but we will need to have a test, that the contract was updated. More than that, it would be great to have a test, that this new contract is also upgradable :)
There was a problem hiding this comment.
I'll add a status_msg contract to show that it does indeed change if we send in a different contract. I'm trying to figure out a way to safeguard against a case where we deploy unwanted contract. In that case, I think we should just validate the core interfaces to see that they still exist like:
- state
- sign
- respond
There was a problem hiding this comment.
+1 to validating interface. We should probably also check that contract is still upgradable, otherwise it's locked
There was a problem hiding this comment.
the contract will always be upgradeable since there's no stopping the contract itself from calling deploy on itself even if the contract is locked
There was a problem hiding this comment.
yes, but if we break the function that is triggering this logic - we will loose the contract
There was a problem hiding this comment.
LGTM.
Just wondering is there anyway we could check that the included wasm code is valid? Because even a benign partner could accidentally propose some bad code.
And what is our remedy if partner gets compromised and propose malicious contract code? But I guess we'd be the only official source to propose update and partners (assuming most are benign) will only vote for our proposed update.
| Some(votes) | ||
| } | ||
|
|
||
| pub fn remove(&mut self, id: &UpdateId) -> Option<Vec<Update>> { |
There was a problem hiding this comment.
So if partner keeps proposing updates and no do_update is done, the updates map will keep piling up? Will that cause issue some day?
There was a problem hiding this comment.
yeah, we need to charge a deposit for propose_update
|
@ppca it's really hard to test valid upgrades. What I'm thinking is checking core interfaces like We can actually make voters pull down the update and verify locally it themselves to see if everything is running correctly. We probably need a CLI tool for this to run through the checks, but right now not a priority |
|
Merging this one for now to let @ppca test the release candidate, will make a separate PR to address the issues noted here |
Terraform Feature Environment Destroy (dev-714)Terraform Initialization ⚙️
|
The contract can now be upgraded or config updated through calling into:
propose_update: (config | wasm) -> UpdateIdvote_update: (UpdateId) -> boolOnly participants from the contract are able to partake. Added config here just to test it can work there too.