Refuse to spend account-mirror boxes via /utxo/transfer while dual-write off (rebased #7994)#8006
Merged
Merged
Conversation
…ite is off _settle_account_transfer_in_utxo closes the account->UTXO half of the #2819 cross-model double-spend, and its docstring states it runs 'independent of UTXO_DUAL_WRITE: a migrated box must be reconciled whenever it exists' -- in the default config the mirror box and the account balance are the same money. The UTXO->account half is unguarded. UTXO_DUAL_WRITE defaults to '0', but the blueprint registers on HAVE_UTXO alone, so /utxo/transfer is live with dual-write off. There apply_transaction() spends the sender's boxes unconditionally while every account-model write sits behind 'if _dual_write:', and nothing in utxo_endpoints.py or utxo_db.py touches account_mirror_boxes. So one self-transfer spends the mirror box, leaves balances untouched, and leaves no unspent mirror box behind the balance: _spend_wallet_mirror joins on spent_at IS NULL, finds 0, and _settle_account_transfer_in_utxo returns early as a 'non-migrated sender' -- the account funds then move again, free. Same double-spend as #2819, reached from the other side, no admin key. Fail closed: with dual-write off there is no account debit to pair the spend with, so migrated funds must move via the account path. The check uses the account_mirror_boxes discriminator, so independently-earned boxes are untouched. Signed-off-by: Scott <scottbphone12@gmail.com>
github-actions
Bot
added
BCOS-L1
Contributor
RTC RewardThis merged PR earned 5 RTC — sent to |
Contributor
|
| Metric | Value |
|---|---|
| Trust Score | 47/100 |
| Certificate ID | BCOS-9ae1c58e |
| Tier | L1 (not met) |
What does this mean?
The BCOS (Beacon Certified Open Source) engine scans for:
- SPDX license header compliance
- Known CVE vulnerabilities (OSV database)
- Static analysis findings (Semgrep)
- SBOM completeness
- Dependency freshness
- Test infrastructure evidence
- Review attestation tier
BCOS v2 Engine - Free & Open Source (MIT) - Elyan Labs
FlintLeng
reviewed
Jul 18, 2026
FlintLeng
left a comment
FlintLeng
left a comment
Contributor
There was a problem hiding this comment.
🔍 PR #8006 审查报告
标题: Refuse to spend account-mirror boxes via /utxo/transfer while dual-write off (rebased #7994)
作者: Scottcjn
钱包地址: RTC019e78d600fb3131c29d7ba80aba8fe644be426e
📊 改动摘要
| 文件 | 新增 | 删除 |
|---|---|---|
node/utxo_endpoints.py |
+42 | 0 |
tests/test_utxo_transfer_spends_account_mirror.py |
+190 | 0 |
| 总计 | +232 | 0 |
🔬 技术评估
1. 问题背景
此 PR 修复了一个跨模型双花漏洞 (#2819)：当 dual-write 处于关闭状态时，account-mirror boxes 可以通过 /utxo/transfer 端点被消费，导致潜在的双花攻击向量。
这是一个严重的安全问题：
- Account-mirror boxes 是跨账本同步的镜像 UTXO
- 在 dual-write 关闭期间，这些 boxes 的状态可能与其他账本不一致
- 允许通过 UTXO 端点消费它们会造成状态不一致和潜在双花
2. 修复方案评估
修复位于 node/utxo_endpoints.py，实现了**安全关闭（fail-closed）**策略：
关键设计点：
├── 使用 account_mirror_boxes 作为判别器
├── 在现有 BEGIN IMMEDIATE 事务中添加参数化 SQL 检查
├── 拒绝时返回 409 Conflict（符合 HTTP 语义）
└── 不影响合法的 UTXO 消费操作
SQL 参数化：正确使用参数化查询，避免 SQL 注入风险。
事务一致性：检查集成在现有 BEGIN IMMEDIATE 事务内，确保原子性。
3. 测试覆盖评估
新增测试文件 tests/test_utxo_transfer_spends_account_mirror.py（+190 行），包含：
- 控制测试：证明合法的 UTXO 消费操作仍然正常工作
- 阻断测试：验证 account-mirror boxes 消费被正确拒绝（409）
- 边界测试：覆盖 dual-write 状态切换场景
测试通过：3/3
4. CI 合规性
PR 添加了 # fetchall-ok: bounded-by-schema 注解，满足 scripts/check_fetchall.sh 的要求，确保 CI 通过。
5. 代码质量
- ✅ 安全优先：fail-closed 设计（拒绝而非允许潜在危险操作）
- ✅ 向后兼容：不影响非 account-mirror 的正常 UTXO 操作
- ✅ 测试充分：包含控制用例和阻断用例
- ✅ SQL 安全：参数化查询
- ✅ 协作注明：明确标注原作者 Vyacheslav-Tomashevskiy
6. 潜在改进建议
- 可考虑添加监控/日志记录被拒绝的消费尝试
- 文档中可明确说明 dual-write 关闭期间的操作限制
✅ 审查结论
APPROVE ✅
这是一个关键安全修复 PR。修复了跨模型双花漏洞，采用安全的 fail-closed 策略，在现有事务框架内正确实现检查逻辑。测试覆盖充分（包含控制用例证明不影响合法操作），代码质量高，符合项目安全标准。
严重性评估：高（安全漏洞修复）
复杂度：中等（涉及并发事务和跨模型状态）
测试覆盖：优秀（控制+阻断+边界）
认领 RTC 奖励: 2 RTC → RTC019e78d600fb3131c29d7ba80aba8fe644be426e
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Rebase of #7994 by @Vyacheslav-Tomashevskiy onto current main, plus the one
# fetchall-ok: bounded-by-schemaannotation the fetchall guard requires (the only thing blocking its CI).Prevents a cross-model double-spend (#2819): account-mirror boxes were spendable from the UTXO side while dual-write is off. Fails closed (409) using the
account_mirror_boxesdiscriminator, parameterized SQL inside the existingBEGIN IMMEDIATE. Verified: the PR's own test passes (3/3, incl. a control proving legit spends still go through), andscripts/check_fetchall.shnow passes.Closes #7994.
Co-authored-by: Vyacheslav-Tomashevskiy
Signed-off-by: Scott scottbphone12@gmail.com