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

lotus msig add-propose #8825

Closed
8 of 18 tasks
kaola526 opened this issue Jun 8, 2022 · 4 comments
Closed
8 of 18 tasks

lotus msig add-propose #8825

kaola526 opened this issue Jun 8, 2022 · 4 comments
Labels
area/multisig Area: Multisig kind/bug Kind: Bug P3 P3: Might get resolved

Comments

@kaola526
Copy link
Contributor

kaola526 commented Jun 8, 2022

Checklist

  • This is not a security-related bug/issue. If it is, please follow please follow the security policy.
  • This is not a question or a support request. If you have any lotus related questions, please ask in the lotus forum.
  • This is not a new feature request. If it is, please file a feature request instead.
  • This is not an enhancement request. If it is, please file a improvement suggestion instead.
  • I have searched on the issue tracker and the lotus forum, and there is no existing related issue or discussion.
  • I am running the Latest release, or the most recent RC(release canadiate) for the upcoming release or the dev branch(master), or have an issue updating to any of these.
  • I did not make any code changes to lotus.

Lotus component

  • lotus daemon - chain sync
  • lotus miner - mining and block production
  • lotus miner/worker - sealing
  • lotus miner - proving(WindowPoSt)
  • lotus miner/market - storage deal
  • lotus miner/market - retrieval deal
  • lotus miner/market - data transfer
  • lotus client
  • lotus JSON-RPC API
  • lotus message management (mpool)
  • Other

Lotus Version

lotus version v1.15.3

Describe the Bug

  1. Create a multi-signature wallet lotus msig create t1gb2sm5ahttih2z4i2hm72o4najql2y2jbxqjwua t1hkurv5y6hb4qpieeddphdq6gxrju7njhbdpcoui t1ifus4ppqvnysgd6wsjbnss7dvi52cfol55hhu7a
  2. Msig Address is t2gimulgw7kd4onsy55h3mshv3ecv6a6kcnnyd2uq
  3. Check for multiple signature wallet addresses lotus msig inspect t2gimulgw7kd4onsy55h3mshv3ecv6a6kcnnyd2uq
Balance: 0 FIL
Spendable: 0 FIL
Threshold: 3 / 3
Signers:
ID      Address
t01136  t1gb2sm5ahttih2z4i2hm72o4najql2y2jbxqjwua
t01134  t1hkurv5y6hb4qpieeddphdq6gxrju7njhbdpcoui
t01001  t1ifus4ppqvnysgd6wsjbnss7dvi52cfol55hhu7a
Transactions:  0
  1. Propose to add a signer t1hkurv5y6hb4qpieeddphdq6gxrju7njhbdpcoui, But,It in signers lotus msig add-propose --from t1gb2sm5ahttih2z4i2hm72o4najql2y2jbxqjwua t2gimulgw7kd4onsy55h3mshv3ecv6a6kcnnyd2uq t1hkurv5y6hb4qpieeddphdq6gxrju7njhbdpcoui
  2. The other signatories agree. lotus msig approve --from t1hkurv5y6hb4qpieeddphdq6gxrju7njhbdpcoui t2gimulgw7kd4onsy55h3mshv3ecv6a6kcnnyd2uq 0 lotus msig approve --from t1ifus4ppqvnysgd6wsjbnss7dvi52cfol55hhu7a t2gimulgw7kd4onsy55h3mshv3ecv6a6kcnnyd2uq 0
  3. Check for multiple signature wallet addresses lotus msig inspect t2gimulgw7kd4onsy55h3mshv3ecv6a6kcnnyd2uq
Balance: 0 FIL
Spendable: 0 FIL
Threshold: 3 / 3
Signers:
ID      Address
t01136  t1gb2sm5ahttih2z4i2hm72o4najql2y2jbxqjwua
t01134  t1hkurv5y6hb4qpieeddphdq6gxrju7njhbdpcoui
t01001  t1ifus4ppqvnysgd6wsjbnss7dvi52cfol55hhu7a
Transactions:  0
  1. Problem: The new signer wallet is already in the signer list, making this process meaningless. We should determine if the signature is in the signature list when we add it

Logging Information

lotus msig create t1gb2sm5ahttih2z4i2hm72o4najql2y2jbxqjwua t1hkurv5y6hb4qpieeddphdq6gxrju7njhbdpcoui t1ifus4ppqvnysgd6wsjbnss7dvi52cfol55hhu7a
sent create in message:  bafy2bzacedptocbvjyeq62pc6kao5aj5enj2xc52alwdmn3mr6mooj4xd3nks
waiting for confirmation..
Created new multisig:  t01137 t2gimulgw7kd4onsy55h3mshv3ecv6a6kcnnyd2uq
lotus msig inspect t2gimulgw7kd4onsy55h3mshv3ecv6a6kcnnyd2uq
Balance: 0 FIL
Spendable: 0 FIL
Threshold: 3 / 3
Signers:
ID      Address
t01136  t1gb2sm5ahttih2z4i2hm72o4najql2y2jbxqjwua
t01134  t1hkurv5y6hb4qpieeddphdq6gxrju7njhbdpcoui
t01001  t1ifus4ppqvnysgd6wsjbnss7dvi52cfol55hhu7a

Repo Steps

  1. lotus msig create t1gb2sm5ahttih2z4i2hm72o4najql2y2jbxqjwua t1hkurv5y6hb4qpieeddphdq6gxrju7njhbdpcoui t1ifus4ppqvnysgd6wsjbnss7dvi52cfol55hhu7a
  2. lotus msig inspect t2gimulgw7kd4onsy55h3mshv3ecv6a6kcnnyd2uq
  3. lotus msig add-propose --from t1gb2sm5ahttih2z4i2hm72o4najql2y2jbxqjwua t2gimulgw7kd4onsy55h3mshv3ecv6a6kcnnyd2uq t1hkurv5y6hb4qpieeddphdq6gxrju7njhbdpcoui
  4. lotus msig approve --from t1hkurv5y6hb4qpieeddphdq6gxrju7njhbdpcoui t2gimulgw7kd4onsy55h3mshv3ecv6a6kcnnyd2uq 0
  5. lotus msig approve --from t1ifus4ppqvnysgd6wsjbnss7dvi52cfol55hhu7a t2gimulgw7kd4onsy55h3mshv3ecv6a6kcnnyd2uq 0
  6. lotus msig inspect t2gimulgw7kd4onsy55h3mshv3ecv6a6kcnnyd2uq
@rjan90 rjan90 added area/multisig Area: Multisig and removed need/triage labels Jun 9, 2022
@rjan90
Copy link
Contributor

rjan90 commented Jun 9, 2022

Hey! Thanks for the issue report. I definelty can see that the adding a check to see if a proposal to add a signer to the msig, is already a member of the msig.

This does not seem to be a harmful bug though, and msig approvers would not necessarily approve this trasaction (due to it being a meaningless transaction), so the priority to fix this might not be the highest 😄

@rjan90 rjan90 added the P3 P3: Might get resolved label Jun 9, 2022
kaola526 added a commit to kaola526/lotus that referenced this issue Jun 9, 2022
@kaola526
Copy link
Contributor Author

kaola526 commented Jun 9, 2022

The question was raised by our testers.
lotus msig add-propose --from t1gb2sm5ahttih2z4i2hm72o4najql2y2jbxqjwua t2gimulgw7kd4onsy55h3mshv3ecv6a6kcnnyd2uq t1hkurv5y6hb4qpieeddphdq6gxrju7njhbdpcoui

ERROR: The add a signer address(t1hkurv5y6hb4qpieeddphdq6gxrju7njhbdpcoui) is included in the signers

@rjan90
Copy link
Contributor

rjan90 commented Jun 9, 2022

Nice! Thanks for the PR @kaola526, it should be reviewed by the devs soon 😄

@rjan90
Copy link
Contributor

rjan90 commented Jul 7, 2022

Closing this issue since the PR has been merged! Thanks 😃

@rjan90 rjan90 closed this as completed Jul 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/multisig Area: Multisig kind/bug Kind: Bug P3 P3: Might get resolved
Projects
None yet
Development

No branches or pull requests

2 participants