Repository navigation
fix(dapp): only allow the genesis action at height 0 - #1407
Conversation
Genesis 动作只允许在创世高度执行,但框架层此前不做任何校验:DriverBase.Exec 只按 action 名派发 Exec_Genesis,height != 0 时是否拒绝完全取决于执行器自己。 coins/coinsx/token 各自有校验,ticket 缺失,于是任意高度都能重入 GenesisInit 造出 isGenesis=true 的 3 段 ticketId 票据,绕过 pubHash 校验、成熟期与提现等待。 新增系统分叉 ForkGenesisOnlyAtHeightZero(默认 0):启用后 DriverBase.Exec 在 派发前统一拒绝 height != 0 的 Genesis 动作并返回 ErrReRunGenesis;配置为 -1 等价于 MaxHeight,保持历史行为以兼容旧区块重放。 - 新增系统分叉必须同步补全 [fork.system],否则 initForkConfig 会让节点启动失败; 本次已更新 cmd/chain33 与 types/testdata 共 6 个配置及合并测试的内联配置 - 单测覆盖 height 0/>0 x 分叉开/关四种组合、分叉高度边界,以及非 Genesis 动作 不受影响
…s fork 接入 chain33 新增的系统分叉 ForkGenesisOnlyAtHeightZero(创世动作 Genesis 只允许 在高度 0 执行,由 DriverBase.Exec 统一拦截): - github.com/33cn/chain33 升到 v1.72.1-0.20261015115410-bc4044f36135:该提交是在 当前 pin 的 v1.72.0 之上只回移本次修复(chain33 分支 fix/genesis-only-height-zero-v1.72.0),避免把 chain33 master 上 30+ 个无关改动 一并带入——实测直接跳到 master 会让 ci_rgbx 的 TSS/DKG 场景稳定超时。上游修复见 33cn/chain33#1407 - 5 个含真实 [fork.system] 的配置补上 ForkGenesisOnlyAtHeightZero=0。chain33 的 initForkConfig 要求每个已注册的系统分叉都必须出现在配置里,缺键或未知键都会让 节点启动失败,因此版本升级与配置补键必须落在同一次提交 - 验证:上述配置均可被新版本 chain33 正常加载;go test ./plugin/dapp/ticket/... 全绿
Review(严格档:实跑 + 变异 + 一轮对抗复核)结论:方向正确、代码层未发现正确性缺陷,可以合入;建议合并前改掉描述里的两处陈述,并补一条测试钉子。守卫"不可绕过"这条我按最强路径攻击过——全仓只有一个 实跑证实(不依赖 PR 自述)
建议合并前改1. 描述里的两处陈述与实现不符(代码是对的,改描述即可)
2. 执行器自身的高度校验失去测试钉住(本 PR 引入的回归保护缺口)
3. 注释写"配置为 -1 … 加载后等价于 MaxHeight",但代码是直接 其余小建议
部署注意
无需改守卫的位置( |
bysomeone
left a comment
There was a problem hiding this comment.
LGTM. 明细与实跑证据见评论:#1407 (comment)
…spatch 处理 #1407 review 的三条合并前建议与两条小建议: - 守卫扩展到本地派发:callLocal 在 ExecLocal_/ExecDelLocal_ 派发前调用同一 checkGenesisHeight,被框架拒绝的创世交易不再为未执行的交易写本地索引。 现有执行器因 CheckReceiptExecOk/回执类型判断本就是空操作,这里补的是框架层 不变量;分叉关闭时行为不变。 - 补钉 coins 执行器自身的高度校验:此前它被框架守卫遮蔽——把 exec.go 的 `if c.GetHeight() == 0` 改成恒真后 TestGenesisRerun 仍全绿。新增 TestGenesisRerunWithoutFrameworkFork 显式关闭框架分叉后再断言,变异验证该 用例会 FAIL,而原用例保持 PASS。 - types/fork_test.go:改用含 ForkGenesisOnlyAtHeightZero=-1 的配置串走 NewChain33Config,真正钉住 initForkConfig 的 -1 -> MaxHeight 转换(原来直接 SetFork(MaxHeight) 绕过了转换,注释与实现不符)。 - 用例统一 TestGenesisGuard* 前缀,-run TestGenesisGuard 可一次跑全,避免 -run TestExecGenesis 漏掉非 Genesis 动作那条。 - 同步 cmd/tools/strategy 的 [fork.system] 模板(先例 296f2d3)。 变异验证:把 checkGenesisHeight 的高度条件去掉后,TestGenesisGuardRejectedAtNonZeroHeight、 TestGenesisGuardRejectedFromForkHeight、TestGenesisGuardLocalDispatchAtNonZeroHeight 全部 FAIL;coins 的 TestGenesisRerunWithoutFrameworkFork 也 FAIL。
|
感谢 review,三条合并前建议 + 两条小建议都已处理(commit 1. 描述里的两处陈述 — 已按实际改掉(PR body)
2. 执行器自身的高度校验失去钉子 — 已补 3. 4. 用例命名 — 已统一为 5. 6. 7. 部署面 — 已按你的建议调整
变异验证:去掉 |
…spatch 处理 #1407 review 的三条合并前建议与两条小建议: - 守卫扩展到本地派发:callLocal 在 ExecLocal_/ExecDelLocal_ 派发前调用同一 checkGenesisHeight,被框架拒绝的创世交易不再为未执行的交易写本地索引。 现有执行器因 CheckReceiptExecOk/回执类型判断本就是空操作,这里补的是框架层 不变量;分叉关闭时行为不变。 - 补钉 coins 执行器自身的高度校验:此前它被框架守卫遮蔽——把 exec.go 的 `if c.GetHeight() == 0` 改成恒真后 TestGenesisRerun 仍全绿。新增 TestGenesisRerunWithoutFrameworkFork 显式关闭框架分叉后再断言,变异验证该 用例会 FAIL,而原用例保持 PASS。 - types/fork_test.go:改用含 ForkGenesisOnlyAtHeightZero=-1 的配置串走 NewChain33Config,真正钉住 initForkConfig 的 -1 -> MaxHeight 转换(原来直接 SetFork(MaxHeight) 绕过了转换,注释与实现不符)。 - 用例统一 TestGenesisGuard* 前缀,-run TestGenesisGuard 可一次跑全,避免 -run TestExecGenesis 漏掉非 Genesis 动作那条。 - 同步 cmd/tools/strategy 的 [fork.system] 模板(先例 296f2d3)。 变异验证:把 checkGenesisHeight 的高度条件去掉后,TestGenesisGuardRejectedAtNonZeroHeight、 TestGenesisGuardRejectedFromForkHeight、TestGenesisGuardLocalDispatchAtNonZeroHeight 全部 FAIL;coins 的 TestGenesisRerunWithoutFrameworkFork 也 FAIL。
…s fork 接入 chain33 新增的系统分叉 ForkGenesisOnlyAtHeightZero(创世动作 Genesis 只允许在 高度 0 执行,由 DriverBase.checkGenesisHeight 在 Exec 与 ExecLocal/ExecDelLocal 派发 前统一拦截): - github.com/33cn/chain33 升到 v1.72.1-0.20261016021757-b12c753ff90b:该提交是在当前 pin 的 v1.72.0 之上只回移本次修复(chain33 分支 fix/genesis-only-height-zero-v1.72.0, 含 #1407 review 的补钉),避免把 chain33 master 上 30+ 个无关改动一并带入——实测直接 跳到 master 会让 ci_rgbx 的 TSS/DKG 场景稳定超时。上游修复见 33cn/chain33#1407 - 5 个含真实 [fork.system] 的配置补上 ForkGenesisOnlyAtHeightZero=0。chain33 的 initForkConfig 要求每个已注册的系统分叉都必须出现在配置里,缺键或未知键都会让节点 启动失败,因此版本升级与配置补键必须落在同一次提交 - 验证:go test ./plugin/dapp/ticket/... 全绿
review 建议把这两点也写进代码注释: - 配置为 0 表示对全部历史高度生效, 启用前必须先确认链上没有高度 != 0 成功执行的创世交易; - 配置里若根本没有 [fork] 段(如 cmd/chain33/bityuan.toml), initForkConfig 被跳过, 该分叉静默按注册默认值生效, 没有缺键/未知键的启动校验提示。
…spatch 处理 #1407 review 的三条合并前建议与两条小建议: - 守卫扩展到本地派发:callLocal 在 ExecLocal_/ExecDelLocal_ 派发前调用同一 checkGenesisHeight,被框架拒绝的创世交易不再为未执行的交易写本地索引。 现有执行器因 CheckReceiptExecOk/回执类型判断本就是空操作,这里补的是框架层 不变量;分叉关闭时行为不变。 - 补钉 coins 执行器自身的高度校验:此前它被框架守卫遮蔽——把 exec.go 的 `if c.GetHeight() == 0` 改成恒真后 TestGenesisRerun 仍全绿。新增 TestGenesisRerunWithoutFrameworkFork 显式关闭框架分叉后再断言,变异验证该 用例会 FAIL,而原用例保持 PASS。 - types/fork_test.go:改用含 ForkGenesisOnlyAtHeightZero=-1 的配置串走 NewChain33Config,真正钉住 initForkConfig 的 -1 -> MaxHeight 转换(原来直接 SetFork(MaxHeight) 绕过了转换,注释与实现不符)。 - 用例统一 TestGenesisGuard* 前缀,-run TestGenesisGuard 可一次跑全,避免 -run TestExecGenesis 漏掉非 Genesis 动作那条。 - 同步 cmd/tools/strategy 的 [fork.system] 模板(先例 296f2d3)。 变异验证:把 checkGenesisHeight 的高度条件去掉后,TestGenesisGuardRejectedAtNonZeroHeight、 TestGenesisGuardRejectedFromForkHeight、TestGenesisGuardLocalDispatchAtNonZeroHeight 全部 FAIL;coins 的 TestGenesisRerunWithoutFrameworkFork 也 FAIL。
|
🎉 This PR is included in version 1.72.6 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
chain33 v1.72.6 已包含系统分叉 ForkGenesisOnlyAtHeightZero(33cn/chain33#1407): - 依赖从 v1.72.0 升到正式版本 v1.72.6,不再需要回移分支/伪版本 - 5 个含真实 [fork.system] 的配置补上 ForkGenesisOnlyAtHeightZero=0。chain33 的 initForkConfig 要求每个已注册的系统分叉都必须出现在配置里,缺键或未知键都会让节点 启动失败,因此版本升级与配置补键必须落在同一次提交 - 已核对 v1.72.0 -> v1.72.6 的 RegisterSystemFork:这期间只新增了这一个 system fork - 验证:上述 5 个配置均可被 v1.72.6 正常加载(无 system fork 报错); go test ./plugin/dapp/ticket/... 全绿
依赖从 v1.72.0 回移版换成 v1.72.7-0.20261016053907-a95e7f042582(chain33 分支 fix/dht-keep-established-identity,33cn/chain33#1408): - v1.72.6 已经包含本 PR 需要的系统分叉 ForkGenesisOnlyAtHeightZero(33cn/chain33#1407), 但仍带着 v1.72.1 引入的回归:钱包就绪后 dht 用钱包空投地址替换节点身份并 reStart(), 运行期更换 peer id; - plugin 的 ci_rgbx 正是被这条路径打断(重启前发现的 TSS peers 与运行身份零交集, auto DKG commit 超时),与创世分叉无关(把它配成 -1 仍复现); - 切到该修复后 ci_rgbx 通过(5m29s,重启前后 peer 名保持不变),因此不再需要 v1.72.0 回移分支,等 chain33 发布 v1.72.7 后可换成正式版本号 - go test ./plugin/dapp/ticket/... 全绿
…commit 依赖从 a95e7f042 跟到 e1bde75e2(v1.72.7-0.20261016082959-e1bde75e25d0): #1408 review 补了真 host 身份稳定性回归、收紧了用例(变异矩阵可拦下收窄/移动守卫), 并把钱包导入的 ErrPrivkeyExist 当成功、去掉名字冲突时多余的 1 秒 sleep。 chain33 依赖最终包含:ForkGenesisOnlyAtHeightZero(33cn/chain33#1407) + dht 身份不再运行期替换(33cn/chain33#1408)。 go test ./plugin/dapp/ticket/... 全绿。
…s fork 接入 chain33 新增的系统分叉 ForkGenesisOnlyAtHeightZero(创世动作 Genesis 只允许在 高度 0 执行,由 DriverBase.checkGenesisHeight 在 Exec 与 ExecLocal/ExecDelLocal 派发 前统一拦截): - github.com/33cn/chain33 升到 v1.72.1-0.20261016021757-b12c753ff90b:该提交是在当前 pin 的 v1.72.0 之上只回移本次修复(chain33 分支 fix/genesis-only-height-zero-v1.72.0, 含 #1407 review 的补钉),避免把 chain33 master 上 30+ 个无关改动一并带入——实测直接 跳到 master 会让 ci_rgbx 的 TSS/DKG 场景稳定超时。上游修复见 33cn/chain33#1407 - 5 个含真实 [fork.system] 的配置补上 ForkGenesisOnlyAtHeightZero=0。chain33 的 initForkConfig 要求每个已注册的系统分叉都必须出现在配置里,缺键或未知键都会让节点 启动失败,因此版本升级与配置补键必须落在同一次提交 - 验证:go test ./plugin/dapp/ticket/... 全绿
依赖从 v1.72.0 回移版换成 v1.72.7-0.20261016053907-a95e7f042582(chain33 分支 fix/dht-keep-established-identity,33cn/chain33#1408): - v1.72.6 已经包含本 PR 需要的系统分叉 ForkGenesisOnlyAtHeightZero(33cn/chain33#1407), 但仍带着 v1.72.1 引入的回归:钱包就绪后 dht 用钱包空投地址替换节点身份并 reStart(), 运行期更换 peer id; - plugin 的 ci_rgbx 正是被这条路径打断(重启前发现的 TSS peers 与运行身份零交集, auto DKG commit 超时),与创世分叉无关(把它配成 -1 仍复现); - 切到该修复后 ci_rgbx 通过(5m29s,重启前后 peer 名保持不变),因此不再需要 v1.72.0 回移分支,等 chain33 发布 v1.72.7 后可换成正式版本号 - go test ./plugin/dapp/ticket/... 全绿
…commit 依赖从 a95e7f042 跟到 e1bde75e2(v1.72.7-0.20261016082959-e1bde75e25d0): #1408 review 补了真 host 身份稳定性回归、收紧了用例(变异矩阵可拦下收窄/移动守卫), 并把钱包导入的 ErrPrivkeyExist 当成功、去掉名字冲突时多余的 1 秒 sleep。 chain33 依赖最终包含:ForkGenesisOnlyAtHeightZero(33cn/chain33#1407) + dht 身份不再运行期替换(33cn/chain33#1408)。 go test ./plugin/dapp/ticket/... 全绿。
问题
「创世动作(
Genesis)只允许在高度 0 执行」此前不是框架不变量,而是各执行器自觉:ErrReRunGenesis框架层
DriverBase.Exec只做funcname := "Exec_" + name反射派发,v1.72.0 与 master 都没有任何 Genesis/height 判断,mempool 与区块校验层也没有。因此任何漏写该校验的执行器都可以在任意高度重入创世逻辑。ticket 正是如此:可重入GenesisInit造出isGenesis=true、3 段ticketId的“创世票”,绕过pubHash校验、成熟期与提现等待,让受害矿工替攻击者挖票并把出块奖励增发到攻击者的ReturnAddress(PoC 在 height=1,700,000 实测成功)。动作名的正确取法:守卫用的是
ExecutorType.DecodePayloadValue返回的 action 名("Genesis")。不要改用Transaction.ActionName()——它经lowcaseFirst返回小写"genesis",写成tx.ActionName() == "Genesis"比较会永不成立、守卫静默失效,而现有用例会全绿。修复
新增系统分叉
ForkGenesisOnlyAtHeightZero(types/fork.go,本 PR 默认 0),抽成DriverBase.checkGenesisHeight(name),在Exec与ExecLocal_/ExecDelLocal_(callLocal)派发前统一拦截:types.ErrReRunGenesis,语义与 coins/coinsx/token 一致-1(initForkConfig加载时转成MaxHeight)时框架不拦截,旧区块可原样重放ExecPack且带一条TyLogErr;MinTxFeeRate > 0时手续费照收(既有手续费行为)。准确说法是不产生任何创世 KV,链上状态不变ExecLocal_/ExecDelLocal_同步跳过:不再为一笔没有执行成功的交易写本地索引(coins 的地址收款统计等);返回空集合而非 error,与CheckReceiptExecOk短路保持一致Genesis的动作,其他动作不受影响(本仓库只有 EVM 自己实现Exec,而 EVM 无 Genesis action)新增系统分叉必须同步补全
[fork.system](initForkConfig对缺失条目会 panic),本 PR 已更新cmd/chain33/chain33.fork.toml、chain33.system.fork.toml、types/testdata/{bityuan,guodun,chain33,local.mvertest}.toml、types/config_merge_test.go内联配置以及cmd/tools/strategy的模板,共 7 处置0。测试
system/dapp/genesis_guard_test.go(新增,8 例,假执行器的Genesis处理函数无条件成功,因此失败只可能来自框架守卫;用例统一TestGenesisGuard*前缀,-run TestGenesisGuard一次跑全):TestGenesisGuardAllowedAtHeightZeroTestGenesisGuardRejectedAtNonZeroHeightErrReRunGenesis,未调用执行器方法TestGenesisGuardNonZeroHeightWithoutForkMaxHeight)时保持历史行为TestGenesisGuardRejectedFromForkHeightTestGenesisGuardNonGenesisActionAtNonZeroHeightTestGenesisGuardWithoutAPITestGenesisGuardLocalDispatchAtNonZeroHeightExecLocal_/ExecDelLocal_Genesis都不派发,高度 0 照常TestGenesisGuardLocalDispatchWithoutFork另补两处测试钉子(review 提出):
system/dapp/coins/executor/exec_genesis_test.go新增TestGenesisRerunWithoutFrameworkFork:显式把框架分叉关成MaxHeight后再断言执行器自身拒绝。否则把exec.go的if c.GetHeight() == 0改成恒真,原TestGenesisRerun仍会被框架层兜住而全绿。types/fork_test.go:用含ForkGenesisOnlyAtHeightZero=-1的配置串走NewChain33Config,钉住initForkConfig的-1 -> MaxHeight转换(原来直接SetFork(MaxHeight)绕过了唯一做转换的地方)。变异验证:去掉
checkGenesisHeight的高度条件后,TestGenesisGuardRejectedAtNonZeroHeight、TestGenesisGuardRejectedFromForkHeight、TestGenesisGuardLocalDispatchAtNonZeroHeight与 coins 的TestGenesisRerunWithoutFrameworkFork全部 FAIL;把exec.go的高度校验改成恒真时,TestGenesisRerun仍 PASS 而新用例 FAIL。go test ./system/dapp/... ./types/全绿,go vet ./system/dapp/ ./types/干净。上线注意(闸门)
TicketAction{Ty:Genesis}),按 0 生效会让这些历史区块重放出不同状态根。扫描通过前不要置 0;存在的话应改为未来高度或-1。[fork]段(例如框架自带的cmd/chain33/bityuan.toml),initForkConfig会被跳过,该分叉静默按注册默认值0对全历史生效——这一层没有任何 panic 提示,需要在发布说明里点明。ForkGenesisOnlyAtHeightZero,否则initForkConfig会让节点启动失败。