Conversation
sidhujag
commented
Mar 4, 2026
sidhujag
commented
- btc checkpoint precompiles
- bootstrap feature
sync 30m from 2h on mainnet
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fd5739b4b2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The install flow deletes the existing chaindata directory before attempting moveOrCopyDir, so a failed copy/rename (for example, cross-device copy failing due to disk full or I/O error) leaves the node with neither the old state nor a complete new state. This is especially dangerous when --syscoin.statebootstrap.force is used on a live datadir, because a transient install failure becomes destructive data loss.
BTCCheckpointLastIndex is mutated in write/delete paths and read in query paths without any mutex/atomic protection, so concurrent block ingestion and RPC/VM reads can race on this shared field. In practice, precompile calls can observe inconsistent values (and on 32-bit builds, torn 64-bit reads are possible), so this counter should be guarded with synchronization similar to other shared chain state.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 17c3e0c945
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
btccheckpointindex.Run currently treats a missing callback as success and returns []byte{}, but this precompile’s contract is 32*k bytes, so callers that decode fixed-width return data will fail on short output. I checked the in-repo contexts that build vm.BlockContext manually (core/vm/runtime/env.go and cmd/evm/internal/t8ntool/execution.go), and they do not populate the new BTC checkpoint callbacks, so this branch is reachable in tooling/tests and yields inconsistent behavior versus node execution where callbacks are always wired.
writeTarGzArchive defers gz.Close()/tw.Close() without checking their errors, so final flush failures (for example, ENOSPC while writing trailer bytes) are silently ignored and the function can return success with a truncated archive. Since createBootstrapArchive then hashes and records that file in the manifest, this can publish a corrupt snapshot as if it were valid; the same pattern exists in writeZipArchive with zw.Close().
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 39b304979f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The new BTC checkpoint callbacks silently fall back to zero/empty when chain does not implement the ad-hoc reader interfaces, which is exactly what happens in eth_call/gas estimation/tracing because internal/ethapi.ChainContext only implements ReadSYSHash, ReadDataHash, and GetNEVMAddress. As a result, precompiles 0x64/0x65/0x66 return zeroed data in RPC simulations while block execution uses real chain data, causing observable behavior mismatches and incorrect simulation results for contracts depending on these precompiles.
This branch applies the Syscoin default bootstrap SHA when no network preset is set, but the no-preset path in geth still uses mainnet defaults elsewhere; that means operators using state bootstrap without --syscoin/--tanenbaum get an unrelated built-in hash enforced and valid archives are rejected unless they manually override --syscoin.statebootstrap.sha256. The default hash should only be auto-selected for explicit Syscoin/Tanenbaum presets.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cba31b051d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
doTest treats --eest as an opt-in for execution-spec tests when deciding fixture downloads, but it never propagates GETH_EEST=1 to the go test subprocess. The newly gated EEST tests in tests/block_test.go, tests/state_test.go, and tests/transaction_test.go only run when that environment variable is set, so build/ci.go test --eest currently downloads fixtures but still skips the intended suite. This breaks the advertised flag behavior and can silently miss EEST regressions.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 397079662e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
When --syscoin.statebootstrap.url is used without --syscoin.statebootstrap.file, the code always downloads to state-bootstrap.tar.gz, and extraction later dispatches by filename extension via build.ExtractArchive. That means a valid .zip bootstrap URL (which the exporter supports) is treated as tar.gz and fails at import time, so URL-only bootstrap is unusable for zip artifacts unless operators discover and set a matching --syscoin.statebootstrap.file path manually.
|
@codex review (dont worry about GETH_ETH_TESTS) focus on others |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e8a68f362a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
exportStateBootstrap currently aborts whenever chaindata/LOCK exists, but os.Stat on the lockfile path is not a liveness check for an active writer. This can reject exports from a stopped node (stale/persistent lockfile path still present), so operators are pushed to --allow-live even when no process is writing, which undermines the safety gate and blocks normal export workflows.
WriteBTCCheckpoint marks the hash as indexed in memory before the enclosing DB batch is committed. If that batch write fails, a retry in the same process will hit the in-memory dedupe path and skip re-writing the checkpoint mappings, leaving disk state missing while runtime state reports the checkpoint as present.
|
@codex review |
|
Codex Review: Didn't find any major issues. Hooray! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
1 participant
