Skip to content
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] Sanity check new threshold/key pair during key updates #5

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

sras
Copy link

@sras sras commented Mar 18, 2020

Sanity check new threshold/key pair during key updates

The current key update function allows following bad values for new threshold/key pair.

  1. Threshold value is Zero, which renders multisig contract without any
    additional security.

  2. Threshold value is larger than the number of keys, which locks
    out the multisig contract completely and renders it useless. This could even lock the target contract where the multisig contract acts as an administrator/owner.

This change adds sanity checks that rejects bad threshold/key pairs as
listed above.

The current key update function allows following bad values.

1. Zero threshold value, when renders multisig contract without any
additional security.

2. Threshold value that is larger than the number of keys, which locks
out the multisig contract completely and renders it useless.

This change adds sanity checks that rejects bad threshold/key pairs as
listed above.
@sras sras force-pushed the sras/sanity-check-key-update branch from 8402d1a to 44f900c Compare April 13, 2020 17:26
@rafoo
Copy link
Contributor

rafoo commented Apr 19, 2020

IMO these checks should be done off-chain. The Tezos client already check this: https://gitlab.com/tezos/tezos/-/blob/master/src/proto_006_PsCARTHA/lib_client/client_proto_multisig.ml#L691.

@sras
Copy link
Author

sras commented Jun 29, 2020

IMO these checks should be done off-chain

@rafoo Why though? Couldn't one can interact with a smart contract via tezos-clients raw contract-call commands (or even via an RPC calls) which, AFAIU does not make any such checks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants