Skip to content

Conversation

@songgaoye
Copy link
Contributor

@songgaoye songgaoye commented Oct 28, 2025

👮🏻👮🏻👮🏻 !!!! REFERENCE THE PROBLEM YOUR ARE SOLVING IN THE PR TITLE AND DESCRIBE YOUR SOLUTION HERE !!!! DO NOT FORGET !!!! 👮🏻👮🏻👮🏻

PR Checklist:

  • Have you read the CONTRIBUTING.md?
  • Does your PR follow the C4 patch requirements?
  • Have you rebased your work on top of the latest master?
  • Have you checked your code compiles? (make)
  • Have you included tests for any non-trivial functionality?
  • Have you checked your code passes the unit tests? (make test)
  • Have you checked your code formatting is correct? (go fmt)
  • Have you checked your basic code style is fine? (golangci-lint run)
  • If you added any dependencies, have you checked they do not contain any known vulnerabilities? (go list -json -m all | nancy sleuth)
  • If your changes affect the client infrastructure, have you run the integration test?
  • If your changes affect public APIs, does your PR follow the C4 evolution of public contracts?
  • If your code changes public APIs, have you incremented the crate version numbers and documented your changes in the CHANGELOG.md?
  • If you are contributing for the first time, please read the agreement in CONTRIBUTING.md now and add a comment to this pull request stating that your PR is in accordance with the Developer's Certificate of Origin.

Thank you for your code, it's appreciated! :)

Summary by CodeRabbit

  • Chores
    • Migrated internal storage to a new module and updated references across the codebase.
    • Removed legacy storage implementations, multi-store plumbing, numerous CLI tools, external sorter, and many related tests.
    • Deleted the protobuf CI workflow used for proto checks and removed some CI test steps.
  • Documentation
    • Removed or consolidated multiple READMEs and changelog entries for legacy storage components.

✏️ Tip: You can customize this high-level summary in your review settings.

@songgaoye songgaoye requested a review from a team as a code owner October 28, 2025 10:02
@songgaoye songgaoye requested review from calvinaco and thomas-nguy and removed request for a team October 28, 2025 10:02
@github-actions

This comment has been minimized.

@github-actions github-actions bot added the build label Oct 28, 2025
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 28, 2025

Walkthrough

Replaces many local memiavl/versiondb/store modules with external github.com/crypto-org-chain/cronos-store/* modules, updates imports and dependency manifests, removes a large set of memiavl/versiondb source files/tests/docs, adjusts the SetupMemIAVL call-site to accept a new chainId argument, and deletes the protobuf CI workflow.

Changes

Cohort / File(s) Summary
Manifests & Dependency
go.mod, gomod2nix.toml, versiondb/go.mod, store/go.mod, memiavl/go.mod
Replaced local github.com/crypto-org-chain/cronos/... module references with github.com/crypto-org-chain/cronos-store/... versions; updated gomod2nix entries; removed some submodule go.mod files.
App & Call-sites
app/app.go, app/bench_test.go, app/versiondb.go, cmd/cronosd/cmd/root.go, cmd/cronosd/cmd/versiondb.go
Updated import paths to cronos-store/*; changed SetupMemIAVL invocation to pass a new chainId argument.
Protos & Generation
proto/memiavl/*, scripts/protocgen.sh, .github/workflows/proto.yml
Updated option go_package in memiavl proto files to cronos-store paths; protocgen script now copies from cronos-store; deleted the protobuf CI workflow file.
memiavl package
memiavl/* (many files: db.go, tree.go, snapshot.go, proof.go, iterator.go, export.go, import.go, mmap.go, layout_*.go, mem_node.go, persisted_node.go, wal.go, types.go, README.md, tests, go.mod, etc.)
Removed nearly the entire memiavl implementation, tests, docs, and module file — including public types/functions for DB, Tree, Snapshot, WAL, iterators, proofs, export/import, layouts, nodes, exporters/importers and many tests.
store config & setup
store/config/*, store/setup.go
Removed memiavl-related config types/constants/TOML template and SetupMemIAVL / setMemIAVL implementations and associated CLI flags.
store implementations
store/cachemulti/store.go, store/memiavlstore/store.go, store/rootmulti/*
Deleted cache wrapper, memiavl-backed store, rootmulti store implementation and related snapshot/import/export and objstore files and tests.
versiondb package
versiondb/* (client, extsort, tsrocksdb, multistore, store, types, streaming_service, tests, README, go.mod)
Removed the entire versiondb package and subpackages: CLI commands, external sorter, tsrocksdb implementation, VersionStore/MultiStore abstractions, streaming service, tests and module file.
CI & Lint
.github/workflows/lint.yml, .github/workflows/build.yml, .github/workflows/proto.yml
Deleted protobuf CI workflow; removed unit-test job from build workflow and trimmed some submodule lint steps.
Changelog
CHANGELOG.md
Added UNRELEASED entry for PR #1895: "feat: use cronos store".

Sequence Diagram(s)

sequenceDiagram
  participant App as App (cronosd)
  participant Setup as SetupMemIAVL (cronos-store)
  participant ExtMod as cronos-store modules

  Note right of App: App.New / initialization
  App->>Setup: SetupMemIAVL(logger, home, appOpts, ..., cacheSize, chainId, baseAppOptions)
  alt memiavl enabled
    Setup->>ExtMod: configure external memiavl/store using chainId
    ExtMod-->>Setup: configured baseApp option(s)
    Setup-->>App: return modified baseAppOptions
  else memiavl disabled
    Setup-->>App: return baseAppOptions unchanged
  end
Loading

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~120 minutes

Areas needing extra attention:

  • All updated import paths (cronos/*cronos-store/*) across packages and go.mod/gomod2nix changes.
  • The changed SetupMemIAVL signature (new chainId parameter) and all callsites.
  • Large-scale removals of memiavl/versiondb code — ensure no remaining callers, interface mismatches, or missing functionality; verify external cronos-store modules provide required APIs.
  • CI workflow deletions that may affect proto generation, unit tests, and coverage reporting.

Possibly related PRs

Suggested labels

cronos, cli

Suggested reviewers

  • thomas-nguy
  • leejw51crypto
  • mmsqe

Poem

🐰 I hopped through folders, swift and spry,

Repointed modules, waved old files goodbye.
Trees and snapshots took a gentle rest,
Cronos-store now takes the quest.
A little hop — a tidy try!

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'feat: use cronos store' directly describes the main change: migrating from the local cronos modules (memiavl, store, versiondb) to external cronos-store modules.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch song/use_cronos_store

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 97e34bb and 2ae79df.

📒 Files selected for processing (1)
  • go.mod (3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
  • GitHub Check: integration_tests (gas)
  • GitHub Check: integration_tests (mint)
  • GitHub Check: integration_tests (upgrade)
  • GitHub Check: integration_tests (ibc_timeout)
  • GitHub Check: integration_tests (ibc_rly_gas)
  • GitHub Check: integration_tests (slow)
  • GitHub Check: integration_tests (ibc_update_client)
  • GitHub Check: integration_tests (ica)
  • GitHub Check: integration_tests (gov)
  • GitHub Check: integration_tests (ibc_rly_evm)
  • GitHub Check: integration_tests (unmarked)
  • GitHub Check: integration_tests (ibc)
  • GitHub Check: Run golangci-lint
  • GitHub Check: build (macos-latest)
  • GitHub Check: build (ubuntu-latest)
  • GitHub Check: build (macos-14)
  • GitHub Check: gomod2nix
  • GitHub Check: Analyze (go)
  • GitHub Check: semgrep-cloud-platform/scan
  • GitHub Check: semgrep-cloud-platform/scan

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@songgaoye songgaoye marked this pull request as draft October 28, 2025 10:04
@socket-security
Copy link

socket-security bot commented Oct 28, 2025

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff Package Supply Chain
Security
Vulnerability Quality Maintenance License
Addedgolang/​github.com/​crypto-org-chain/​cronos-store/​versiondb@​v0.0.0-20251119030934-19a930cec475100100100100100
Addedgolang/​github.com/​crypto-org-chain/​cronos-store/​store@​v0.0.0-20251119030934-19a930cec475100100100100100

View full report

@songgaoye songgaoye marked this pull request as ready for review November 18, 2025 09:18
@github-actions github-actions bot added the ci label Nov 19, 2025
@thomas-nguy
Copy link
Collaborator

Can you change the PR target to be main branch?

@songgaoye songgaoye force-pushed the song/use_cronos_store branch from 77721c2 to aff8700 Compare November 19, 2025 13:23
@songgaoye songgaoye changed the base branch from release/v1.5.x to main November 19, 2025 13:24
@songgaoye
Copy link
Contributor Author

sure

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6d8020b and b3084c6.

⛔ Files ignored due to path filters (1)
  • go.sum is excluded by !**/*.sum
📒 Files selected for processing (2)
  • go.mod (3 hunks)
  • gomod2nix.toml (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (22)
  • GitHub Check: Socket Security: Pull Request Alerts
  • GitHub Check: integration_tests (mint)
  • GitHub Check: integration_tests (upgrade)
  • GitHub Check: integration_tests (ica)
  • GitHub Check: integration_tests (gas)
  • GitHub Check: integration_tests (ibc_timeout)
  • GitHub Check: integration_tests (slow)
  • GitHub Check: integration_tests (gov)
  • GitHub Check: integration_tests (ibc_update_client)
  • GitHub Check: integration_tests (ibc)
  • GitHub Check: integration_tests (ibc_rly_evm)
  • GitHub Check: integration_tests (ibc_rly_gas)
  • GitHub Check: integration_tests (unmarked)
  • GitHub Check: Run golangci-lint
  • GitHub Check: build (macos-14)
  • GitHub Check: build (ubuntu-latest)
  • GitHub Check: unittest
  • GitHub Check: build (macos-latest)
  • GitHub Check: gomod2nix
  • GitHub Check: Analyze (go)
  • GitHub Check: semgrep-cloud-platform/scan
  • GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (1)
gomod2nix.toml (1)

259-270: ****

The claim of a version mismatch is inaccurate. Go.mod contains a replace directive that explicitly redirects github.com/crypto-org-chain/cronos-store/memiavl from the constraint v0.0.4 to the replacement version v0.0.0-20251119030934-19a930cec475, which matches the gomod2nix.toml entries. Replace directives take precedence in Go module resolution—no mismatch exists and no correction is needed.

Likely an incorrect or invalid review comment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants