feat: support multisig transfers - #15
Conversation
|
Follow-up self-review fixes in 541b134:
Verification rerun:
|
180ac6e to
cedab34
Compare
Findings[P2] Legacy 转账无法使用主网型
|
There was a problem hiding this comment.
Findings
[P2] 校验按 trace 查到的 transfer 是否与本次请求一致 — cmd/transfer/multisig.go:410
ReadTransfer 成功后,代码只检查响应非 nil,便把当前 legacy multisig transfer 判为已完成;没有核对返回快照的 TraceID、AssetID、Amount、收款方或 memo。--trace 是用户可指定的,因此只要当前账户曾把同一 trace 用于另一笔 transfer,本命令就会打印 transaction already completed 并直接成功退出,即使本次资产、金额、来源 multisig 或目标完全不同。这样会把未执行的转账报告为已完成。应在返回成功前,把快照中可用的交易身份字段与 input、sender group 和 receiver 做一致性校验;不一致时应明确报错,而不是按完成处理。
已验证 Example: 新增的 TestResolveLegacyMultisigTransactionSkipsOutputsWhenTraceExists 传入 TransferInput{AssetID: "asset", Amount: 1, TraceID: "trace"},但 mock 仅返回 Snapshot{TraceID: "trace"},其 AssetID 为空、Amount 为 0,也没有任何收款方信息;resolveLegacyMultisigTransaction 仍返回 UTXOStateSpent。这直接验证了不匹配或信息不足的响应会被接受为已完成。
Review scope
仅审查 cedab34de7629eb7efb12e3bdb79a88651bdc5b9...50b52113e2a7c370d15b1c0bc48507aa335cab03 的增量,共涉及 README.md、cmd/transfer/multisig.go 和 cmd/transfer/multisig_test.go。同时核对了 transfer 命令入口、legacy multisig 的 output 查找与交易匹配逻辑,以及依赖 SDK 中 ReadTransfer、Snapshot 和错误码判断的实现。
Validation
git diff --check cedab34de7629eb7efb12e3bdb79a88651bdc5b9...50b52113e2a7c370d15b1c0bc48507aa335cab03:通过。go test ./cmd/transfer:通过。go vet ./cmd/transfer:通过。
Residual risks
未连接真实 Mixin API 验证已完成 legacy multisig transfer 的快照字段形态;本次判断基于仓库锁定的 SDK 实现、类型定义和单元测试行为。
|
Review notes follow-up
Verification after the latest fix:
All passed. |
Summary
--sendersand--sender-thresholdsource flags--traceas create-or-join identity: create and sign a new transfer, or continue signing an existing oneCommands
mixin-cli transfer ... --trace ... --senders ... --sender-threshold ...mixin-cli transfer cancel ...(aliases:unlock,cancel-signature)mixin-cli transfer cancel-request --request ...mixin-cli safe transfer ... --trace ... --senders ... --sender-threshold ...mixin-cli safe transfer cancel --trace ...(aliases:unlock,cancel-signature)For legacy transfers, every signer supplies the same transfer fields because the legacy API has no lookup-by-trace endpoint. Existing Safe requests can be continued with
--tracealone.Test coverage
Added focused tests for deterministic signer ordering, exact-balance input selection, signed-output recovery, request and raw-transaction validation, malformed external UTXOs, duplicate/non-member signers, JSON receiver decoding, amount precision, and command registration.
Verification
go test ./...go test -race -vet=off ./...go vet ./cmd/safe ./cmd/transfergo buildplus CLI help smoke testsgit diff --check origin/master...HEADSelf-review also fixed a legacy output-selection TOCTOU race and a Safe create/read race before this PR was opened.
Documentation
README.md: documented legacy and Safe multisig transfer, join-signing, signature cancellation, and legacy request cancellation workflows.