-
Notifications
You must be signed in to change notification settings - Fork 235
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Problem: app hash mismatch in block-stm #1539
Conversation
Solution: - try to reproduce in integration test first
WalkthroughThe recent changes enhance a Go project's configuration and dependency management. The Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1539 +/- ##
===========================================
+ Coverage 15.24% 36.12% +20.88%
===========================================
Files 67 97 +30
Lines 4874 7725 +2851
===========================================
+ Hits 743 2791 +2048
- Misses 4037 4585 +548
- Partials 94 349 +255 |
Co-authored-by: mmsqe <mavis@crypto.com> Signed-off-by: yihuang <huang@crypto.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
might need a change log?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (1)
integration_tests/test_basic.py (1)
942-976
: LGTM!The new test case
test_block_stm_delete
effectively validates the functionality of block-stm by addressing a specific bug.Suggestion: Add a timeout to the
w3_wait_for_block
function call.- w3_wait_for_block(w3, w3.eth.block_number + 3, timeout=30) + w3_wait_for_block(w3, w3.eth.block_number + 3, timeout=30)
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
go.sum
is excluded by!**/*.sum
Files selected for processing (4)
- go.mod (3 hunks)
- gomod2nix.toml (1 hunks)
- integration_tests/configs/default.jsonnet (2 hunks)
- integration_tests/test_basic.py (2 hunks)
Additional comments not posted (8)
integration_tests/configs/default.jsonnet (2)
26-30
: LGTM!The addition of the
memiavl
configuration block with parametersenable
,zero-copy
, andsnapshot-interval
enhances memory management capabilities.
51-51
: LGTM!The modification of the
block-stm-workers
parameter from 16 to 32 improves concurrency for block processing.go.mod (3)
90-90
: LGTM!The removal of
github.com/cosmos/cosmos-proto v1.0.0-beta.5
from the mainrequire
block indicates that the project no longer directly relies on this version of thecosmos-proto
library.
90-90
: LGTM!The reintroduction of
github.com/cosmos/cosmos-proto v1.0.0-beta.5
as an indirect dependency ensures compatibility or functionality through transitive dependencies.
100-100
: LGTM!The version update for
github.com/crypto-org-chain/go-block-stm
fromv0.0.0-20240408011717-9f11af197bde
tov0.0.0-20240806075927-09a64748f883
may include bug fixes, new features, or other improvements.integration_tests/test_basic.py (1)
28-37
: LGTM!The addition of new functions
derive_new_account
,sign_transaction
, andw3_wait_for_block
expands the capabilities of the test cases, enhancing the testing framework.gomod2nix.toml (2)
217-217
: Verify the correctness of the hash update.The hash for
github.com/crypto-org-chain/go-block-stm
has been updated tosha256-DuqcnTqpUVRxV+I+CaBuDg/1CLmMeXdw4VEFeuCsUbU=
. Ensure that this new hash is correct and matches the module's integrity.
216-216
: Verify the compatibility and stability of the version update.The version for
github.com/crypto-org-chain/go-block-stm
has been updated tov0.0.0-20240806075927-09a64748f883
. Ensure that this new version is compatible and stable with the rest of the codebase.
This reverts commit 903bc2b.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- integration_tests/configs/default.jsonnet (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- integration_tests/configs/default.jsonnet
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Solution:
👮🏻👮🏻👮🏻 !!!! REFERENCE THE PROBLEM YOUR ARE SOLVING IN THE PR TITLE AND DESCRIBE YOUR SOLUTION HERE !!!! DO NOT FORGET !!!! 👮🏻👮🏻👮🏻
PR Checklist:
make
)make test
)go fmt
)golangci-lint run
)go list -json -m all | nancy sleuth
)Thank you for your code, it's appreciated! :)
Summary by CodeRabbit
New Features
memiavl
with options for enabling, zero-copy, and snapshot intervals.block-stm-workers
for enhanced performance.Bug Fixes
go-block-stm
.Chores