Repository navigation
fix(p2p): keep the established dht identity instead of replacing it at runtime - #1408
Conversation
…t runtime v1.72.1 修好空投索引取值范围后(53d304ea8 等),doGenAirDropKey 里“用钱包空投地址 替换节点身份并 reStart()”这条路径开始真的执行: - waitPid=false 时 initP2P 先生成并持久化一个随机身份,host 用它对外发布 peer id; - 钱包就绪后 doGenAirDropKey 取到钱包派生身份,与 addrbook 不同就 saveKey + reStart, 于是节点在运行期换了 peer id。 任何按 peer id 配置了本节点的对端都会失效。plugin 的 ci_rgbx 就是这样被打断的: 它在重启前发现 4 个 para 节点的 peer 名写进 TSS peers 配置,钱包就绪后节点身份变化, 配置里的 peers 与实际运行身份零交集,DKG 无法形成,auto DKG commit 必然超时 (实测 t=20s/60s 看到的 peer 名与配置零交集;关掉本仓库的创世分叉仍复现,与本分叉无关)。 修复:节点身份一旦在 addrbook 里持久化就不再在运行期替换;钱包派生身份只在节点还没有 身份时采用(waitPid=true 时会在 host 启动前确定身份,原语义不变)。 测试:新增 system/p2p/dht/p2p_airdrop_identity_test.go - 已有身份且钱包身份不同 → 不替换、不请求重启、addrbook 不变 - 无身份 → 采用钱包身份 - 两边身份相同 → 保持原状 变异验证:去掉该分支后 TestGenAirDropKeyKeepsEstablishedIdentity 失败。 go test ./system/p2p/... 全绿。
依赖从 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/... 全绿
plugin 侧验证(修复前后对照,同一份 plugin 代码)用带诊断分支(重启前后各打印一次 peer 名,超时导出容器日志)指向本提交跑 修复前(chain33 v1.72.6)— 失败,且 peer 身份确实被换掉: 修复后(本提交)— 通过,身份保持不变:
另外确认与本仓库已合入的创世分叉无关:把
|
结论:可合这 11 行守卫的必要性已被独立复现:去掉它,真 host 的 peer id 会在钱包解锁后约 1.1 秒变化(三次运行全部复现, 建议在本 PR 内补的两处测试
表述上的四处更正
两点说明(不要求改动)
另外两点(供参考)
覆盖边界审了 |
bysomeone
left a comment
There was a problem hiding this comment.
LGTM. 详细结论与两处补测试的建议见评论:#1408 (comment)
libangzhu
left a comment
There was a problem hiding this comment.
建议批准。这个改动是对的,没有必须改完才能合的问题。
这个 PR 在修什么
waitPid=false 时,节点先用一把随机私钥启动,这个 peer id 已经告诉别的节点了。等钱包准备好,旧代码会把 peer id 换成助记词推导出来的那把钥匙,然后重启。v1.72.1 把空投地址的索引修好之后,这条路才真的走到。peer id 换一次,之前按旧 id 配好的连接就全失效。评论里 ci_rgbx 的 TSS 超时就是这个原因。空投索引写入钱包后会一直返回同一把私钥,所以只换这一次。
这次改完,addrbook 里已经有私钥就保持不动,不再换、也不再重启。waitPid=true 并且 addrbook 还没有私钥时,仍会在 host 启动前把助记词私钥写进去。
两种节点升级后会怎样
已经换成助记词 peer id 的节点,升级后还是这把钥匙,不会退回原来的随机私钥。
addrbook 里只存一把私钥。当初替换的时候,助记词私钥已经把随机私钥盖掉了,然后节点才重启。现在对外用的是助记词 peer id,说明磁盘上存的就是这把。升级后读出来继续用。原来的随机私钥如果还在,只在钱包账户 dht node award 里,不会再写回 addrbook。
waitPid=false 时,随机私钥一旦写进 addrbook,这个 peer id 就固定了。后来钱包里有了助记词,也不会再换。
没有私钥时,节点先生成一把随机私钥并存下来,再用它启动。钱包就绪后,只是把这把随机私钥复制进钱包(账户名 dht node award),方便备份。助记词推导的空投地址也会在钱包里建出来,但它只是钱包里的另一个账户,不会变成节点的 peer id。
这把钥匙一直有效的前提是 addrbook 里这条记录还在、还能读出来。记录被删掉或者损坏时,下次启动会再生成一把新的随机私钥,仍然不会改用助记词。
建议改一下,不改也能合
每次重启会多一次多余的导入。 身份固定之后,addrbook 里是随机私钥,钱包空投账户是另一把钥匙,两把永远对不上,所以每次启动都会再去钱包里导入一次节点私钥。
// system/p2p/dht/p2p.go:509-528
if savePrivkey != "" && !p.p2pCfg.WaitPid {
var parm types.ReqWalletImportPrivkey
parm.Privkey = savePrivkey
parm.Label = "dht node award"
for {
_, err = p.api.ExecWalletFunc("wallet", "WalletImportPrivkey", &parm)
if err == types.ErrLabelHasUsed {
parm.Label = fmt.Sprintf("node award %d", rand.Int31n(1024000))
time.Sleep(time.Second)
continue
}
break
}
}钱包先看账户名,再看这把私钥在不在:
// wallet/wallet_proc.go:516-549
if Account != nil && err == nil {
walletlog.Error("ProcImportPrivKey Label is exist in wallet!")
return nil, types.ErrLabelHasUsed
}
// ...
if existDec := wcom.CBCDecrypterPrivkey([]byte(wallet.Password), existPriv); len(existDec) == len(privkeybyte) && string(existDec) == string(privkeybyte) {
walletlog.Error("ProcImportPrivKey Privkey is exist in wallet!")
return nil, types.ErrPrivkeyExist
}第一次启动,名字还没用过,导入成功。以后每次启动只再走两步就退出,不会一直循环,也不挡已经启动的 host:名字 dht node award 已占用,睡 1 秒再换一个新名字;私钥上次已经导入过,就结束。每次重启多等 1 秒,日志里多两条看起来像失败的 Error。其它导入错误在这里被直接丢掉,p2p 没有日志。这把随机私钥助记词推不出来,第一次导入如果真失败,钱包里就没有备份。
建议把「私钥已经在钱包里」当成成功,名字冲突时不要先睡 1 秒,其它错误打日志:
_, err = p.api.ExecWalletFunc("wallet", "WalletImportPrivkey", &parm)
if err == nil || err == types.ErrPrivkeyExist {
break
}
if err == types.ErrLabelHasUsed {
parm.Label = fmt.Sprintf("node award %d", rand.Int31n(1024000))
continue
}
log.Error("genAirDropKey", "WalletImportPrivkey err", err)
break测试还差一点点。 「两把私钥相同」那个用例在比较时就返回了,没走到这次新增的判断。「已有身份不替换」那个用例,如果创建空投账户失败,也会返回不替换,测试照样能过,而且没有检查钱包导入是否真的被调用。建议把这两个调用断言出来。再补一条:waitPid=true 但 addrbook 里已经有另一把私钥时,也不替换。
两处旧注释会误导人。 一处还写着「先用临时 peer id,等钱包建好再换」。另一处还写着「保证最后用空投地址」。按这次的行为,addrbook 里已经存下的身份不再换。注释改成这句话就行。
…ixture 处理 #1408 review(bysomeone / libangzhu): 测试补强(三位 reviewer 都点到现有三个用例拦不住收窄/移动守卫): - 新增真 host 回归 TestP2PHostIdentityStableAfterWalletReady:真 queue + 真 host + 真 leveldb addrbook + 钱包 mock 返回另一把钥,钱包就绪后轮询 host.ID() 与 addrbook 身份。去掉守卫时 1.2s 内 FAIL(与 review 实测的 ~1.1s 一致),而既有 Test_p2p 仍通过。 - 新增 TestGenAirDropKeyKeepsEstablishedIdentityWithWaitPid:waitPid=true 且 addrbook 已有另一把钥时同样不得替换 —— 收窄成 `savePrivkey != "" && !WaitPid` 时该用例 FAIL。 - 前两个用例分别断言 WalletImportPrivkey 被调用/未被调用,把“守卫必须在导入之后”变成 契约:跳过导入块(等价于守卫上提)时 KeepsEstablishedIdentity FAIL。 - TestGenAirDropKeySameIdentityNoRestart 改名为 ...SameKeyReturnsEarly 并注明它钉的是 同密钥提前返回,不是本次守卫。 代码(libangzhu 建议): - 钱包导入把 ErrPrivkeyExist(身份已在钱包里)当成功,名字冲突时不再 sleep 1 秒, 其它错误打日志后放弃;避免修复后每次重启多等 1 秒并刷出像失败的 Error。 注释(bysomeone 更正): - initP2P 的“生成临时 pid,待创建钱包之后替换”已不成立,改为“身份一经发布不再更换, 钱包就绪后只把它导入钱包备份”。 - 守卫注释去掉“waitPid=true 原语义不变”,写明真实条件:waitPid=false 时 initP2P 已 Randkey() 落盘身份,这类节点不会采用钱包身份;只有 addrbook 无身份且 host 启动前 拿到空投地址(waitPid=true)才用它作身份。 线索更正:v1.72.1 里重新落地空投索引的是 ec54763 / 563e80d / d2a16d3; 最早的 53d304e 在 11 分钟后被 b890d1d 回滚,回滚信息原文即 "Reverts the airdrop index change so an existing node's peer ID stays the same" —— 说明“改索引 = 改已有节点 peer id”当时已被认识到,重新落地时没把守卫带回来。 变异矩阵(三处变异全部被拦): - 守卫去掉 → KeepsEstablishedIdentity / WithWaitPid / 真 host 用例 FAIL - 守卫收窄为 && !WaitPid → WithWaitPid FAIL - 跳过钱包导入块 → KeepsEstablishedIdentity FAIL go test ./system/p2p/... 全绿(dht 包连跑两遍稳定)。
|
感谢两位的详细 review。逐条处理如下(commit 测试建议(bysomeone 1、2;libangzhu「测试还差一点点」)
表述更正(bysomeone 1–4)
libangzhu 的其余建议
两点说明的处理
|
…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/... 全绿。
|
🎉 This PR is included in version 1.72.7 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
依赖从 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/... 全绿。
问题
v1.72.1 重新落地空投索引修复后,
doGenAirDropKey里「用钱包空投地址替换节点身份并reStart()」这条路径开始真的执行:waitPid=false(bityuan.toml / chain33.toml 默认)时,initP2P先Randkey()生成并持久化一个身份,host 用它对外发布 peer id;doGenAirDropKey取到钱包派生身份,与 addrbook 不同就saveKey(钱包身份)+reStart()—— 节点在运行期换了 peer id。索引演化的准确线索(review 更正):
53d304ea8airDropIndex(纳秒+pid);11 分钟后被b890d1d38回滚,回滚信息原文:"Reverts the airdrop index change so an existing node's peer ID stays the same" —— 当时就已知「改索引 = 改已有节点 peer id」ec5476309/563e80d77/d2a16d33bec5476309起randIndex恒落在[1e8, 1.01e8]),没有再处理身份问题因此:≤ v1.72.0 时索引约 99% 越界(
isValidIndex拒绝),替换路径事实上不可达,身份稳定;v1.72.1 起替换是确定性的 —— 每个新节点在钱包解锁后约 1 秒必换一次,一个钱包只换一次(空投索引 sticky)。影响:任何按 peer id 配置了本节点的对端都会失效(peering 列表、TSS peers、白名单)。plugin
ci_rgbx即被此打断。修复
节点身份一旦在 addrbook 里持久化(peer id 已对外发布),就不再在运行期用钱包空投地址替换;钱包派生身份只在节点还没有身份时采用。
waitPid=true且 addrbook 为空时,仍在 host 启动前用空投地址确定身份(原语义保留)。同时按 review 收尾:
ErrPrivkeyExist(身份已在钱包里)当成功,名字冲突时不再sleep 1s,其它错误打日志后放弃 —— 修复后「addrbook 身份 ≠ 钱包空投键」成为常态,避免每次重启多等 1 秒并刷出像失败的 Error;initP2P与导入块的过时注释,并按 review 更正守卫注释(真实条件是waitPid=false时initP2P已落盘身份 ⇒ 这类节点不会采用钱包身份)。测试
system/p2p/dht/p2p_airdrop_identity_test.go:TestP2PHostIdentityStableAfterWalletReadyhost.ID()与 addrbook 身份不变。去掉守卫时 1.2s 内 FAIL(既有Test_p2p仍通过,这正是该缺陷能上线的原因)TestGenAirDropKeyKeepsEstablishedIdentityWalletImportPrivkey被调用(守卫必须在导入之后)TestGenAirDropKeyKeepsEstablishedIdentityWithWaitPidwaitPid=true+ 已有另一把身份 → 同样不替换(收窄守卫会 FAIL)TestGenAirDropKeyAdoptsWalletKeyWithoutIdentity!WaitPid门控)TestGenAirDropKeySameKeyReturnsEarly变异矩阵(review 指出原三个用例拦不住,现已全部拦下):
...KeepsEstablishedIdentity、...WithWaitPid、真 host 用例 FAILsavePrivkey != "" && !WaitPid...WithWaitPidFAIL...KeepsEstablishedIdentityFAILgo test ./system/p2p/...全绿(dht 包连跑两遍稳定)。验证(证据等级说明)
plugin 侧用临时诊断分支(重启前后各打印 peer 名,超时导出容器日志)指向本提交跑
ci_rgbx:t=20s/60s看到的 peer 名与写进 TSS peers 配置的零交集 →auto DKG commit timeout;auto DKG done,ci_rgbxpass 5m29s;ForkGenesisOnlyAtHeightZero配成-1后仍复现。证据等级:以上来自 CI job 日志与本提交上的可执行用例;
ci_rgbxharness 另有若干已文档化的成因会产生同一条失败串,本 PR 未逐个排除其历史偶发(仓内rgbx-e2e.log不是现场日志,未作为依据)。未覆盖
plugin/p2p/gossip/p2p.go有一份同构的「 ResetPeerkey + ReStart」运行期换身份实现,未被本次修复覆盖;当前配置均为types=["dht"],暂时不受影响,另案处理。