fix: restore PRs inadvertently reverted by #3039 squash-merge#3070
fix: restore PRs inadvertently reverted by #3039 squash-merge#3070blindchaser merged 6 commits intomainfrom
Conversation
## Describe your changes and provide context Moved transaction generation off of the main benchmark thread for the cryptosim benchmark. ## Testing performed to validate your change Tested locally. --------- Signed-off-by: Cody Littley <cody.littley@seinetwork.io> Co-authored-by: Cody Littley <cody.littley@seinetwork.io>
This commit was inadvertently reverted by the squash-merge of #3039. Restores the tx ordering check (firstCosmosSeen), len(evmEntries)>0 and len(v2Entries)>0 guards in ProcessTXsWithOCCGiga. Conflicts with #3050's slog migration resolved by using package-level logger instead of ctx.Logger(). Made-with: Cursor
This commit was inadvertently reverted by the squash-merge of #3039. Restores ReceiptStoreConfig in app.toml, readReceiptStoreConfig(), BackendTypeName(), receipt config tests, and init_test.go. Conflicts resolved: - app_config.go: kept both ReceiptStore (from #3035) and Admin (from #3062) - receipt_store.go: kept BackendTypeName but used #3050's slog convention - parquet_store_test.go: kept #3053's deterministic pruning test fix Made-with: Cursor
This commit was inadvertently reverted by the squash-merge of #3039. Restores LatticeHashEnabled config option in StateCommitConfig, composite store lattice hash support, and related tests. Conflict resolved: store_test.go needed all three imports (metrics from HEAD, evm+logger from #3043). Made-with: Cursor
This commit was inadvertently reverted by the squash-merge of #3039. Restores the benchmark config changes (BlocksPerCommit=1, SnapshotInterval=1000, SnapshotMinTimeInterval=60). The consoleLogger addition from #3046 is dropped because #3050 replaced the logger package with slog, making it unnecessary. Made-with: Cursor
|
The latest Buf updates on your PR. Results from workflow Buf / buf (pull_request).
|
The cherry-picked tests from #3035 and #3043 still referenced the old logger package (removed by #3050 slog migration). Fix: - composite/store_test.go: remove logger import/arg, add CommittedRootHash to mock - parquet/store_config_test.go: remove dbLogger arg from NewStore - receipt/parquet_store_test.go: remove dbLogger arg from NewReceiptStore Made-with: Cursor
![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: 2d3a7ab912
ℹ️ 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".
| // Builds blocks and sends them to the blocks channel. | ||
| func (b *blockBuilder) mainLoop() { | ||
| for { | ||
| block := b.buildBlock() |
There was a problem hiding this comment.
blockBuilder.mainLoop calls buildBlock() before it checks ctx.Done(), so shutdown can race with block generation. During CryptoSim teardown, the data generator is closed (its RNG is nulled) without waiting for the builder goroutine; if cancellation happens mid-build, later BuildTransaction calls can dereference a nil RNG and panic in a background goroutine. Add cancellation checks during block construction or explicitly synchronize builder shutdown before closing shared state.
Useful? React with 👍 / 👎.
| m.startDataDirSizeSampling(dataDir, config.BackgroundMetricsScrapeInterval) | ||
| } | ||
| m.startProcessIOSampling(config.BackgroundMetricsScrapeInterval) | ||
| m.startUptimeSampling(time.Now()) |
There was a problem hiding this comment.
Uptime sampling is started in NewCryptosimMetrics at construction time, but NewCryptoSim constructs metrics before account/contract setup runs. When setup is long (e.g., large prepopulation or restore), exported uptime includes non-workload initialization time, which skews benchmark-derived rates and makes run-to-run throughput comparisons inaccurate. The uptime sampler should be started once setup finishes, not during metric object creation.
Useful? React with 👍 / 👎.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #3070 +/- ##
==========================================
+ Coverage 58.35% 58.39% +0.04%
==========================================
Files 2080 2081 +1
Lines 171659 171790 +131
==========================================
+ Hits 100170 100324 +154
+ Misses 62564 62536 -28
- Partials 8925 8930 +5
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
arajasek
left a comment
arajasek
left a comment
There was a problem hiding this comment.
Confirmed my PR is back!
|
Late to the party but LGTM; thank you for getting this sorted so quickly @blindchaser 🙌 |
* main: fix(giga): match v2 correctness checks (#3071) Added clone method to canned random (#3076) Helper files for the flatKV cache implementation (#3072) fix: restore PRs inadvertently reverted by #3039 squash-merge (#3070) Refine logging to avoid printing expensive objects on hot path (#3066) Fix flaky tendermint syncer test (#3065) Add runtime log level control via gRPC admin service (#3062) chore: dcoument run RPC suite on legacy vs giga (#3041) chore: self-contained revert tests, contract reorg, and failure analysis (#3033)
5 participants
Summary
The squash-merge of #3039 (
feat(flatkv): add read-only LoadVersion for state sync) inadvertently reverted changes from 5 previously-merged PRs. Theyiren/flatkv-readonlybranch had accumulated stale versions of files through merge-from-main commits, and when the final squash-merge landed, those stale versions overwrote the newer code onmain.PRs reverted by #3039 and restored in this PR
firstCosmosSeentx ordering check,len(evmEntries) > 0/len(v2Entries) > 0guards inProcessTXsWithOCCGigaReceiptStoreConfigin app.toml,readReceiptStoreConfig(),BackendTypeName(), receipt config testsEnableLatticeHashconfig, composite store lattice hash supportblock.go,block_builder.gofor cryptosim benchmarkBlocksPerCommit=1,SnapshotInterval=1000,SnapshotMinTimeInterval=60Conflict resolutions
Since several PRs landed after #3039 (notably #3050 slog migration, #3053 flaky test fix, #3062 admin service), cherry-picks required manual conflict resolution:
app/app.go(fix(giga): check whether txs follow Giga ordering #2810): Kept fix(giga): check whether txs follow Giga ordering #2810's structural changes, usedlogger.Error(from Replace all loggers with package levelslog#3050) instead ofctx.Logger().Errorcmd/seid/cmd/app_config.go(Add receiptdb config option in app.toml #3035): Kept bothReceiptStore(from Add receiptdb config option in app.toml #3035) andAdmin(from Add runtime log level control via gRPC admin service #3062)sei-db/ledger_db/receipt/receipt_store.go(Add receiptdb config option in app.toml #3035): RestoredBackendTypeNamebut dropped logger param (superseded by Replace all loggers with package levelslog#3050 slog)sei-db/ledger_db/receipt/parquet_store_test.go(Add receiptdb config option in app.toml #3035): Kept Fix flaky test: TestParquetFilePruning #3053's deterministic pruning test fixsei-db/state_db/sc/composite/store_test.go(Add config to enable lattice hash #3043): Merged all three needed importssei-db/common/logger/logger.go(Add console logger and fix memiavl config for benchmark #3046): Kept deletion from Replace all loggers with package levelslog#3050;consoleLoggeris no longer needed with slogsei-db/state_db/bench/wrappers/db_implementations.go(Add console logger and fix memiavl config for benchmark #3046): Kept Replace all loggers with package levelslog#3050's no-logger-param API