Skip to content

fix(multisig): recompute confirmation weights from current owners - #1344

Open
vipwzw wants to merge 1 commit into
masterfrom
fix-multisig-weight-recompute
Open

vipwzw wants to merge 1 commit into
masterfrom
fix-multisig-weight-recompute

Conversation

@vipwzw

@vipwzw vipwzw commented Oct 19, 2026

Copy link
Copy Markdown
Collaborator

问题

isConfirmed 累加的是确认时写入 ConfirmedOwner 的权重快照,而 owner 的 add/del/modify/replace 不会回头修正已存在的确认。因此 owner 被删除或降权后,其历史确认仍按旧权重计入阈值:一笔在 owner 变更前未达阈值的 pending 转账,可以在 owner 集合变更后“凑够”阈值被执行,即使用了当前 owner 集合并未授权的旧确认。

典型场景:多签成员密钥泄露,其余 owner 将其移出/降权后,该成员此前对某笔转账的确认仍然有效。

修复

  • isConfirmed 增加 multiSigAcc 与 useCurWeight 参数;ForkMultiSigFixStaleWeight 生效后按当前 owner 列表重新取权重,已删除 owner 直接跳过。
  • MultiSigConfirmTx、executeTransferTx、executeAccOperateTx、executeOwnerOperateTx 全部走新逻辑。
  • 分叉前保留旧的快照逻辑,默认高度 0 生效。
  • 新增单元测试覆盖“owner 被删除/降权后旧确认不再生效”。

验证

  • go build ./plugin/dapp/multisig/... 通过
  • go test ./plugin/dapp/multisig/executor/ -count=1 通过

isConfirmed summed the Weight snapshots stored in ConfirmedOwner when each
owner confirmed. Owner add/del/modify/replace operations never invalidate
those snapshots, so after an owner is removed or demoted their stale
confirmation still counts at the old weight and a pending transfer can reach
the threshold with approvals the current owner set no longer supports.

Behind ForkMultiSigFixStaleWeight, isConfirmed now re-reads each confirming
owner's current weight from the multisig account and skips owners that have
been removed. The pre-fork path keeps the old snapshot logic for consensus
compatibility. Add a unit test covering removed and demoted owners.
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.

1 participant