fboucquez / symbol-bootstrap

A tool that allows you to quickly configure and setup Symbol testnets and nodes.
Apache License 2.0
47 stars 28 forks source link

feat: convert to or modify multisig account added #307

Closed yilmazbahadir closed 2 years ago

yilmazbahadir commented 2 years ago

symbol-bootstrap modifyMultisig command is added. It's useful to convert main account into a multisig account or to modify the multisig account structure. Takes the following parameters, the user will be prompted for the unset parameters.

-A, --addressAdditions=addressAdditions              Cosignatory accounts addresses to be added (separated by a
                                                       comma).

  -D, --addressDeletions=addressDeletions              Cosignatory accounts addresses to be removed (separated by a
                                                       comma).

  -a, --minApprovalDelta=minApprovalDelta              Number of signatures needed to approve a transaction. If the
                                                       account is already multisig, enter the number of cosignatories to
                                                       add or remove.

  -r, --minRemovalDelta=minRemovalDelta                Number of signatures needed to remove a cosignatory. If the
                                                       account is already multisig, enter the number of cosignatories to
                                                       add or remove.
yilmazbahadir commented 2 years ago

Please also run npm run style:fix to update the index.ts and docs

I have done the following testing:

Multisig M, cosigners A,B,C , ServiceOperatorAccount S. It looks really nice:

  1. Convert to multisig M using M private key. It creates a bonded where All A,B,C need to cosign.
  2. Modify mutlisg M using A cosigner private key. It creates a bonded where some of B,C to cosign
  3. Convert to multisig M using S private key. It creates a bonded where all M, A, B, C need to cosign (wallet warning shown as expected)
  4. Modify multisig M using S private key. It creates a bonded where some A, B, C need to cosign (wallet warning shown as expected)

Could the command ask (optionally) for cosigners private keys. Use case, I'm a regular user that wants to convert my main account to multisig for security reasons. I could provide my cosigners private keys on the spot without bonded, using full aggregate complete. This could be an improvement though, the service providers should never know any cosigner private key.

Great! Will run the style:fix script. Let's implement this improvement with a different PR, created https://github.com/symbol/symbol-bootstrap/issues/308. Currently writing some unit tests and also adding some parameter sanity checks, will be updating the PR shortly.

sonarcloud[bot] commented 2 years ago

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 3 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication