Skip to content

fix(ethrpc): prevent uint64→int64 overflow in AssembleChain33Tx - #3

Closed
bysomeone wants to merge 627 commits into
masterfrom
fix/evm-uint64-overflow-in-transfer
Closed

fix(ethrpc): prevent uint64→int64 overflow in AssembleChain33Tx#3
bysomeone wants to merge 627 commits into
masterfrom
fix/evm-uint64-overflow-in-transfer

Conversation

@bysomeone

Copy link
Copy Markdown
Owner

Summary

同步修复 eth_sendRawTransaction 入口中同类型的 uint64→int64 溢出隐患。

Changes

  • tx.go:AssembleChain33Tx — bigAmount.Int64() 前增加 IsInt64() 检查,溢出时返回 nil
  • eth.go:SendRawTransaction — 增加 nil 保护,防止 AssembleChain33Tx 失败时 panic

🤖 Generated with Claude Code
Co-Authored-By: Claude noreply@anthropic.com

vipwzw and others added 29 commits May 14, 2026 13:21
平行链可以开启收取手续费
Recomputed from latest coverage_tn.out, excluding *.pb.go files
(go tool cover -func reports 65.5% statement coverage).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Establish docs/superpowers/ as the standard location for design specs
and implementation plans produced via the Claude + superpowers workflow.

- README.md explains the brainstorm → spec → plan → execute → verify flow
- specs/ and plans/ subdirectories with .gitkeep placeholders
- Include the spec and plan for this Claude collaboration setup itself

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Document chain33-specific conventions Claude (and humans new to the
codebase) need to know: Makefile-wrapped git workflow, GOPATH path
constraint, GOPROXY for CN users, layered module map, testing
discipline, and pitfall list.

The conventions cover the chain33 specifics that aren't visible from
code alone — for instance, `make branch` / `make push` wrap git,
`types/*.pb.go` shouldn't be hand-edited, and `.travis.yml` /
`.gitlab-ci.yml` should not be touched without confirmation.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Two corrections from code review of the initial CLAUDE.md commit:

- 模块计数:目录速查列了 18 个代码模块,文首误标为 19;修正
  并说明 build/ 与 docs/ 不算代码模块
- gg18 TSS 实际位于 system/crypto/tss/gg18/,不在 p2p/ 下;
  移除 p2p 段的错误括注,改在 system/ 段补充指引

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Add team-shared Claude Code permissions for chain33. Allowlist covers:
- make targets that are read-only or safely scoped (test, race, lint,
  coverage, fmt, depends, build, cli) — excludes push/sync/docker
- go subcommands that don't mutate remote state
- git read-only commands (status, diff, log, show, blame)
- search/navigation utilities (ls, find, grep, rg, wc, file)
- gh read commands (pr/issue/run view & list)

Commands that mutate shared state (git push, make push, gh pr create,
git reset --hard) stay outside the allowlist and prompt every time.

Adjust .gitignore so .claude/settings.json (team config) is tracked
while .claude/settings.local.json (personal overrides) stays ignored.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Three small corrections from the final code review:

- Add coverage_*.out and build/coverage/ to .gitignore so the claim in
  CLAUDE.md ("已在 .gitignore") becomes factually true; also stops these
  test artifacts from cluttering `git status`.
- Align workflow stage naming: CLAUDE.md said "TDD" where the rest of
  the docs say "execute"; change to "execute(TDD)" to keep TDD visible
  while matching docs/superpowers/README.md.
- Add a back-reference from docs/superpowers/README.md to CLAUDE.md so
  contributors landing on the README know where the entry point is.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The old badge used a stale camo-proxied URL whose hex payload pointed
to golang/gddo's status.svg on godoc.org (now shut down). The image no
longer loads. Replace with the current pkg.go.dev badge pointing at
this repo.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Establish docs/superpowers/ as the standard location for design specs
and implementation plans produced via the Claude + superpowers workflow.

- README.md explains the brainstorm → spec → plan → execute → verify flow
- specs/ and plans/ subdirectories with .gitkeep placeholders
- Include the spec and plan for this Claude collaboration setup itself

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Document chain33-specific conventions Claude (and humans new to the
codebase) need to know: Makefile-wrapped git workflow, GOPATH path
constraint, GOPROXY for CN users, layered module map, testing
discipline, and pitfall list.

The conventions cover the chain33 specifics that aren't visible from
code alone — for instance, `make branch` / `make push` wrap git,
`types/*.pb.go` shouldn't be hand-edited, and `.travis.yml` /
`.gitlab-ci.yml` should not be touched without confirmation.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Two corrections from code review of the initial CLAUDE.md commit:

- 模块计数:目录速查列了 18 个代码模块,文首误标为 19;修正
  并说明 build/ 与 docs/ 不算代码模块
- gg18 TSS 实际位于 system/crypto/tss/gg18/,不在 p2p/ 下;
  移除 p2p 段的错误括注,改在 system/ 段补充指引

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Add team-shared Claude Code permissions for chain33. Allowlist covers:
- make targets that are read-only or safely scoped (test, race, lint,
  coverage, fmt, depends, build, cli) — excludes push/sync/docker
- go subcommands that don't mutate remote state
- git read-only commands (status, diff, log, show, blame)
- search/navigation utilities (ls, find, grep, rg, wc, file)
- gh read commands (pr/issue/run view & list)

Commands that mutate shared state (git push, make push, gh pr create,
git reset --hard) stay outside the allowlist and prompt every time.

Adjust .gitignore so .claude/settings.json (team config) is tracked
while .claude/settings.local.json (personal overrides) stays ignored.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Three small corrections from the final code review:

- Add coverage_*.out and build/coverage/ to .gitignore so the claim in
  CLAUDE.md ("已在 .gitignore") becomes factually true; also stops these
  test artifacts from cluttering `git status`.
- Align workflow stage naming: CLAUDE.md said "TDD" where the rest of
  the docs say "execute"; change to "execute(TDD)" to keep TDD visible
  while matching docs/superpowers/README.md.
- Add a back-reference from docs/superpowers/README.md to CLAUDE.md so
  contributors landing on the README know where the entry point is.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…bility)

Fix critical bugs found during systematic code review including:
- Remote crash vectors (DER parsing panic, MAVL proof nil deref, download nil deref)
- Security issues (webhook RCE, AES nonce/IV reuse, ethrpc auth bypass)
- Consensus risks (map iteration non-determinism, executor cache logic error)
- Resource leaks (ethrpc Close() not stopping HTTP server)
- Concurrency bugs (queue double-close, WaitGroup race, tasks slice race)
- Correctness (account overflow, error swallowing, CORS method typo)

Each fix includes a regression test demonstrating the bug pattern.
See docs/bugfix.md for detailed descriptions of all findings.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
为 chain33-cli account 子命令增加 k2addr:直接根据 hex 私钥推导 BTC/ETH 地址,
无需启动 RPC 节点。

  ./chain33-cli account k2addr -k <hex> [-t 0|1|2]

addressType: btc(0), btcMultiSign(1), eth(2)

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Go 1.23 锁定了 linker 对 //go:linkname 的使用 (cmd/link 默认开启
-checklinkname=1),fjl/memsize 通过 //go:linkname 拿到 runtime.stopTheWorld,
触发 "invalid reference to runtime.stopTheWorld" 链接错误,导致 Go 1.23+
工具链下 chain33 编译失败。

go-ethereum 在 PR #30253 (2024-08-02 合并) 移除了 memsize 依赖,首个稳定
发行版本是 v1.14.8。本提交把 ethereum/go-ethereum 从 v1.12.0 升到 v1.14.8,
连同其传递依赖一起升级 (protobuf v1.31→v1.34, golang.org/x/net v0.17→v0.24
等),从根上去掉 -checklinkname=0 这种绕过 workaround 的需求。

rpc/jrpchandler_test.go: protobuf v1.34 在某些路径上会写 sizeCache 内部字段,
导致 testify mock 用 `On("SendTx", tx)` 做按值比较时与运行时的 tx 实例不等。
改用 mock.MatchedBy + proto.Equal 做语义比较。

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
7198f44 改 CBCEncrypterPrivkey 使用随机 IV (修 F-WAL-002 IV 复用),
随之产生两处后续问题,本提交一起修掉:

1. wallet/wallet_proc.go: ProcImportPrivKey 通过比较 ciphertext hex
   字符串判断私钥是否已存在;随机 IV 后同一私钥每次加密结果不同,
   导致 `Account.Privkey == Encrypteredstr` 永远不等,把"已存在"误判
   成 ErrPrivkey。改成解密 Account.Privkey 后与原始 privkey bytes 比较。

   这也是 TestAll → testProcImportPrivkeysFile2 在 PR 分支上回归
   (master 通过) 的根因。

2. wallet/common/common_test.go: 测试原本用 16 字节私钥;新格式
   `IV(16) + ciphertext(len(privkey))` 下 16 字节输入会让"new format"
   解密分支返回非 32 字节结果而回退到 legacy,断言失败。改成 32 字节
   私钥 (匹配 secp256k1 生产实际),并把固定长度断言从 `len(privkey)`
   改为 `aes.BlockSize + len(privkey)`。

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- common/db/go_pegasus.go: wrap deferred Delete bench write in a closure
  so time.Since(start) measures elapsed time, not 0 (defer evaluates
  args at registration time)
- queue/exec_map_order_bug_test.go: gofmt key-value alignment
- system/mempool/mempool_test.go: drop pointless append() over single
  slice value (vet "append with no values" warning)
- system/p2p/dht/protocol/download/wgdone_bug_test.go: remove
  unreachable wg.Done() line; comment explains why the test
  deliberately reproduces the buggy pattern via panic

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
go-ethereum v1.14.8 (upgraded in 3ae8ca5) requires Go 1.21+ — the
toolchain directive in go.mod becomes "unknown directive: toolchain"
when parsed by 1.20, which broke the build-with-latest-stable-go matrix
job and the AppVeyor build.

- .github/workflows/build.yml: bump the only hardcoded version
  (`go-version: '1.20'`) to 1.21. All other jobs already read from
  `go-version-file: 'go.mod'`.
- appveyor.yml: switch the hardcoded C:\go120 GOROOT/PATH to AppVeyor's
  `stack: go 1.21` directive, which auto-installs the requested Go
  toolchain regardless of image preset.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The check_fmt job runs ineffassign/gosec via golangci-lint v1.55.2,
whose bundled go/types cannot parse Go 1.24's stdlib export data
("unsupported version: 2"). The toolchain go1.24.5 directive (auto-
added by go mod tidy on a 1.24 host) forces CI to download 1.24 even
though setup-go installed 1.21 — breaking the lint pass with
spurious typecheck failures across packages.

Drop it. go 1.21 alone is enough for go-ethereum v1.14.8; the
toolchain directive was incidental, not required.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- queue/rpc_close_leak_bug_test.go: add ReadHeaderTimeout to the test
  http.Server to satisfy gosec G112 (Slowloris).
- queue/webhook_rce_bug_test.go: annotate the env-var-sourcing
  placeholder string with #nosec G101; the value is not a real
  credential.
- types/evm_event.pb.go: regroup imports (stdlib first) per goimports
  v0.24.0 — pre-existing generated-file drift that newer goimports
  flags as needing 'make fmt'.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
queue/download_tasks_race_bug_test.go declared `index` and `name`
fields on the local task struct but never read them, which
golangci-lint structcheck flags as unused. Access them in the read
loops where the original code already touches the slice — the test
remains a race-detection demonstration, just with the field accesses
made explicit.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The concurrent-sign section judged pass/fail by a 60s wall-clock budget
(select + time.After). Under -race plus per-sign Paillier-2048 keygen on a
shared CI runner it intermittently overran the budget. The same commit
passed gg18 on the push-event run and failed on the pull_request-event run
of PR 33cn#1361 -- a classic flaky signature.

Replace the wall-clock budget with a structural guarantee:
- drop the select{ <-c / time.After(60s) } gate; use wg.Wait(), so the
  test body no longer has any path that fails due to elapsed time
- correctness is decided solely by verifySignatureWithDKG (cryptographic
  verify), which is time-independent
- reduce concurrent signs 10 -> 4 to shrink the -race CPU long tail
- widen the remaining timeouts into deadlock-only backstops, centralized
  as named constants (peerWait/dkg/sign/reshare/barrier/childExit)

Verified locally: -race x10 all PASS, no data race, ~15.4s; gofmt/go vet ok.

Design: docs/superpowers/specs/2026-06-08-gg18-flaky-fix-design.md

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The test deliberately reproduced an "Add after goroutine launch" race
(go func(){ wg.Done() }(); wg.Add(1)) to demonstrate F-BCSYNC-001, but its
recover() sat in the OUTER goroutine while the panic happened in the INNER
child goroutine it spawned (push_wg_bug_test.go:30). Go's recover does not
cross goroutines, so the negative-WaitGroup panic escaped and crashed the
whole queue test binary whenever scheduling ran Done() before Add() --
intermittently failing the unit-test CI job (same commit passed on the
pull_request run, failed on the push run).

It also had no real value: blockchain/push.go already uses the correct
Add-before-go pattern (Add at :576, goroutine at :577), so the demonstrated
bug does not exist in the source; the test exercised only Go's sync.WaitGroup,
never chain33 code, and its sole assertion was assert.True(t, true).

Verified: queue package passes 20/20 runs after removal.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…rgence

TestConnManager called FetchNearestPeers(1) immediately after
RoutingTable().TryAddPeer and then indexed peers[0]. The routing table is
eventually consistent, so FetchNearestPeers intermittently returned an empty
slice; require.NotNil passed (an empty slice is non-nil) but peers[0] panicked
with "index out of range [0] with length 0", crashing the unit-test CI job
(same commit passed on the pull_request run, failed on the push run).

Replace the immediate read with require.Eventually that polls until the routing
table reflects h2, then asserts. Pre-existing flaky test, not introduced by this
PR; conns.go was not the cause.

Verified: TestConnManager passes 10/10 locally.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
First fix (poll FetchNearestPeers for 5s) still failed intermittently
(--- FAIL: TestConnManager 5.10s). Root cause: h2 is a plain libp2p host that
does NOT run the DHT protocol, so h1's DHT background maintenance evicts h2 from
the routing table after the one-shot TryAddPeer; a read-only poll then never
sees it again.

Re-add h2 inside the poll loop (each tick TryAddPeer then read immediately) and
widen the timeout to 30s, so a transient eviction is recovered on the next tick.
Pre-existing flaky test, not introduced by this PR.

Verified 5/5 locally (the flaky does not reproduce locally; CI is the real check).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
fix: address 20 bugs from codebase review
Same overflow pattern as the EVM plugin fix — bigAmount.Int64()
and uint64(amount) conversions without range check. Add IsInt64()
guard and nil protection in SendRawTransaction caller.

Co-Authored-By: Claude <noreply@anthropic.com>
@bysomeone bysomeone closed this Jul 30, 2026
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.

6 participants