ws4charlie commented 1 week ago

Description

The maximum length of inbound message is defined as const MaxMessageLength = 10240 in the zetacore. If for some reason (it is impossible at the moment), an inbound transaction carries a message longer message, the PostVoteInbound RPC call in observer code below will return an error (due to ValidateBasic failure), which result in endless rescan of the tx.

To avoid potential endless tx rescan, zetaclient should SKIP invalid inbound vote message.

// post inbound vote message to zetacore
for _, event := range events {
    msg := ob.GetInboundVoteFromBtcEvent(event)
    if msg != nil {
        _, err = ob.PostVoteInbound(ctx, msg, zetacore.PostVoteInboundExecutionGasLimit)
        if err != nil {
        return errors.Wrapf(err, "error PostVoteInbound") // we have to re-scan this block next time
        }
    }
}

How Has This Been Tested?

[x] Tested CCTX in localnet
[ ] Tested in development environment
[x] Go unit tests
[ ] Go integration tests
[ ] Tested via GitHub Actions

Summary by CodeRabbit

Release Notes

New Features
- Whitelist functionality for SPL tokens on Solana.
- Integration of SPL deposits and withdrawals.
- Enhanced processability checks for inbound events.
- New methods for decoding memos and determining event processability.
Improvements
- Enhanced logging and error handling in inbound event processing.
- Improved build reproducibility for releases.
- Comprehensive testing framework with new tests for various scenarios.
Bug Fixes
- Resolved issues with message registration and handling compliance violations.
- Improved error handling for out-of-gas errors during omnichain calls.

coderabbitai[bot] commented 1 week ago

[!IMPORTANT]

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

📝 Walkthrough

## Walkthrough The pull request introduces a comprehensive update to the ZetaChain project, featuring new functionalities such as whitelisting SPL tokens on Solana, improved build reproducibility, and enhanced testing frameworks for concurrent operations. Key refactoring efforts include the removal of the HSM signer from the zetaclient and adjustments to the zetaclientd CLI. Several fixes address issues related to message registration, peer discovery, and error handling during omnichain calls. The overall changes focus on enhancing functionality, improving code quality, and addressing various bugs across different components. ## Changes | File Path | Change Summary | |------------------------------------------------|----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| | changelog.md | Updated to reflect new features, improvements, refactoring, and fixes, including whitelisting SPL tokens, SPL deposits/withdrawals, and enhanced testing frameworks. | | x/crosschain/types/message_vote_inbound.go | Comment updated for `WithMemoRevertOptions` to `WithRevertOptions`, no logic change. | | zetaclient/chains/bitcoin/observer/event.go | Removed `InboundProcessability` enum, updated `Processability` and `CheckEventProcessability` methods to use `clienttypes.InboundProcessability`. | | zetaclient/chains/bitcoin/observer/event_test.go | Renamed test function from `Test_CheckProcessability` to `Test_Processability`, updated expected types in test cases to `clienttypes.InboundProcessability`. | | zetaclient/chains/bitcoin/observer/inbound.go | Enhanced `GetInboundVoteFromBtcEvent` method logic to include message validation before returning. | | zetaclient/chains/bitcoin/observer/inbound_test.go | Added new test cases for fee rate calculations and error handling, expanded coverage for transaction scenarios. | | zetaclient/chains/solana/observer/inbound.go | Refactored `BuildInboundVoteMsgFromEvent` method to improve logging and error handling, added `CheckEventProcessability` method for better processability checks. | | zetaclient/chains/solana/observer/inbound_test.go | Added `Test_CheckEventProcessability`, updated `Test_BuildInboundVoteMsgFromEvent` to reflect new validation logic. | | zetaclient/compliance/compliance.go | Removed `DoesInboundContainsRestrictedAddress` function and associated imports, streamlining compliance checks. | | zetaclient/types/event.go | Introduced new `InboundProcessability` enum and methods for `DecodeMemo` and `Processability` in `InboundEvent`. | | zetaclient/types/event_test.go | Added unit tests for `InboundEvent` type, validating both decoding and processability logic. | ## Possibly related PRs - **#2795**: The implementation of Solana restricted address is related to the main PR's focus on enhancing functionality and security in transaction handling, particularly with respect to deposits and withdrawals. - **#3149**: The fix to abort CCTX on dust amount in revert outbound transactions directly relates to the main PR's updates on handling transaction states and error conditions. - **#3162**: The fix to skip depositor fee calculation on irrelevant transactions aligns with the main PR's emphasis on improving transaction processing efficiency and error handling. - **#3155**: The fix to avoid panic in inscription parsing is relevant as it addresses error handling in the context of Bitcoin transactions, which is a focus area in the main PR. - **#3082**: The refactor to replace the Docker-based inscription builder with a Golang implementation enhances the functionality related to Bitcoin inscriptions, which is a key aspect of the main PR's updates. ## Suggested labels `no-changelog`, `CONSENSUS_BREAKING_ACK` ## Suggested reviewers - fbac - kingpinXD - swift1337 - lumtis - skosito - brewmaster012

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?

❤️ Share

- [X](https://twitter.com/intent/tweet?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A&url=https%3A//coderabbit.ai) - [Mastodon](https://mastodon.social/share?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A%20https%3A%2F%2Fcoderabbit.ai) - [Reddit](https://www.reddit.com/submit?title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&text=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code.%20Check%20it%20out%3A%20https%3A//coderabbit.ai) - [LinkedIn](https://www.linkedin.com/sharing/share-offsite/?url=https%3A%2F%2Fcoderabbit.ai&mini=true&title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&summary=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code)

🪧 Tips

### Chat There are 3 ways to chat with [CodeRabbit](https://coderabbit.ai): - Review comments: Directly reply to a review comment made by CodeRabbit. Example: - `I pushed a fix in commit , please review it.` - `Generate unit testing code for this file.` - `Open a follow-up GitHub issue for this discussion.` - Files and specific lines of code (under the "Files changed" tab): Tag `@coderabbitai` in a new review comment at the desired location with your query. Examples: - `@coderabbitai generate unit testing code for this file.` - `@coderabbitai modularize this function.` - PR comments: Tag `@coderabbitai` in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples: - `@coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.` - `@coderabbitai read src/utils.ts and generate unit testing code.` - `@coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.` - `@coderabbitai help me debug CodeRabbit configuration file.` 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 using PR comments) - `@coderabbitai pause` to pause the reviews on a PR. - `@coderabbitai resume` to resume the paused reviews. - `@coderabbitai review` to trigger an incremental review. This is useful when automatic reviews are disabled for the repository. - `@coderabbitai full review` to do a full review from scratch and review all the files again. - `@coderabbitai summary` to regenerate the summary of the PR. - `@coderabbitai resolve` resolve all the CodeRabbit review comments. - `@coderabbitai configuration` to show the current CodeRabbit configuration for the repository. - `@coderabbitai help` to get help. ### Other keywords and placeholders - Add `@coderabbitai ignore` anywhere in the PR description to prevent this PR from being reviewed. - Add `@coderabbitai summary` to generate the high-level summary at a specific location in the PR description. - Add `@coderabbitai` anywhere in the PR title to generate the title automatically. ### Documentation and Community - Visit our [Documentation](https://docs.coderabbit.ai) for detailed information on how to use CodeRabbit. - Join our [Discord Community](http://discord.gg/coderabbit) to get help, request features, and share feedback. - Follow us on [X/Twitter](https://twitter.com/coderabbitai) for updates and announcements.

codecov[bot] commented 1 week ago

Codecov Report

Attention: Patch coverage is 70.79646% with 33 lines in your changes missing coverage. Please review.

Project coverage is 62.06%. Comparing base (771317f) to head (fe26d13). Report is 1 commits behind head on develop.

Files with missing lines	Patch %	Lines
zetaclient/chains/bitcoin/signer/signer.go	0.00%	18 Missing :warning:
zetaclient/chains/solana/observer/inbound.go	87.09%	3 Missing and 1 partial :warning:
zetaclient/chains/evm/observer/v2_inbound.go	0.00%	3 Missing :warning:
...taclient/chains/evm/observer/v2_inbound_tracker.go	0.00%	3 Missing :warning:
zetaclient/types/event.go	89.65%	2 Missing and 1 partial :warning:
zetaclient/chains/bitcoin/observer/event.go	92.30%	1 Missing :warning:
zetaclient/chains/bitcoin/observer/inbound.go	50.00%	1 Missing :warning:

Additional details and impacted files

[![Impacted file tree graph](https://app.codecov.io/gh/zeta-chain/node/pull/3184/graphs/tree.svg?width=650&height=150&src=pr&token=JABAIMO8MP&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=zeta-chain)](https://app.codecov.io/gh/zeta-chain/node/pull/3184?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=zeta-chain) ```diff @@ Coverage Diff @@ ## develop #3184 +/- ## =========================================== - Coverage 62.38% 62.06% -0.33% =========================================== Files 427 428 +1 Lines 30416 30438 +22 =========================================== - Hits 18974 18890 -84 - Misses 10596 10708 +112 + Partials 846 840 -6 ``` | [Files with missing lines](https://app.codecov.io/gh/zeta-chain/node/pull/3184?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=zeta-chain) | Coverage Δ | | |---|---|---| | [x/crosschain/types/message\_vote\_inbound.go](https://app.codecov.io/gh/zeta-chain/node/pull/3184?src=pr&el=tree&filepath=x%2Fcrosschain%2Ftypes%2Fmessage_vote_inbound.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=zeta-chain#diff-eC9jcm9zc2NoYWluL3R5cGVzL21lc3NhZ2Vfdm90ZV9pbmJvdW5kLmdv) | `100.00% <ø> (ø)` | | | [zetaclient/chains/base/observer.go](https://app.codecov.io/gh/zeta-chain/node/pull/3184?src=pr&el=tree&filepath=zetaclient%2Fchains%2Fbase%2Fobserver.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=zeta-chain#diff-emV0YWNsaWVudC9jaGFpbnMvYmFzZS9vYnNlcnZlci5nbw==) | `85.36% <100.00%> (+0.25%)` | :arrow_up: | | [zetaclient/compliance/compliance.go](https://app.codecov.io/gh/zeta-chain/node/pull/3184?src=pr&el=tree&filepath=zetaclient%2Fcompliance%2Fcompliance.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=zeta-chain#diff-emV0YWNsaWVudC9jb21wbGlhbmNlL2NvbXBsaWFuY2UuZ28=) | `14.28% <ø> (+3.86%)` | :arrow_up: | | [zetaclient/chains/bitcoin/observer/event.go](https://app.codecov.io/gh/zeta-chain/node/pull/3184?src=pr&el=tree&filepath=zetaclient%2Fchains%2Fbitcoin%2Fobserver%2Fevent.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=zeta-chain#diff-emV0YWNsaWVudC9jaGFpbnMvYml0Y29pbi9vYnNlcnZlci9ldmVudC5nbw==) | `95.65% <92.30%> (-0.10%)` | :arrow_down: | | [zetaclient/chains/bitcoin/observer/inbound.go](https://app.codecov.io/gh/zeta-chain/node/pull/3184?src=pr&el=tree&filepath=zetaclient%2Fchains%2Fbitcoin%2Fobserver%2Finbound.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=zeta-chain#diff-emV0YWNsaWVudC9jaGFpbnMvYml0Y29pbi9vYnNlcnZlci9pbmJvdW5kLmdv) | `32.92% <50.00%> (+0.86%)` | :arrow_up: | | [zetaclient/chains/evm/observer/v2\_inbound.go](https://app.codecov.io/gh/zeta-chain/node/pull/3184?src=pr&el=tree&filepath=zetaclient%2Fchains%2Fevm%2Fobserver%2Fv2_inbound.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=zeta-chain#diff-emV0YWNsaWVudC9jaGFpbnMvZXZtL29ic2VydmVyL3YyX2luYm91bmQuZ28=) | `0.00% <0.00%> (ø)` | | | [...taclient/chains/evm/observer/v2\_inbound\_tracker.go](https://app.codecov.io/gh/zeta-chain/node/pull/3184?src=pr&el=tree&filepath=zetaclient%2Fchains%2Fevm%2Fobserver%2Fv2_inbound_tracker.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=zeta-chain#diff-emV0YWNsaWVudC9jaGFpbnMvZXZtL29ic2VydmVyL3YyX2luYm91bmRfdHJhY2tlci5nbw==) | `0.00% <0.00%> (ø)` | | | [zetaclient/types/event.go](https://app.codecov.io/gh/zeta-chain/node/pull/3184?src=pr&el=tree&filepath=zetaclient%2Ftypes%2Fevent.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=zeta-chain#diff-emV0YWNsaWVudC90eXBlcy9ldmVudC5nbw==) | `89.65% <89.65%> (ø)` | | | [zetaclient/chains/solana/observer/inbound.go](https://app.codecov.io/gh/zeta-chain/node/pull/3184?src=pr&el=tree&filepath=zetaclient%2Fchains%2Fsolana%2Fobserver%2Finbound.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=zeta-chain#diff-emV0YWNsaWVudC9jaGFpbnMvc29sYW5hL29ic2VydmVyL2luYm91bmQuZ28=) | `35.77% <87.09%> (+4.52%)` | :arrow_up: | | [zetaclient/chains/bitcoin/signer/signer.go](https://app.codecov.io/gh/zeta-chain/node/pull/3184?src=pr&el=tree&filepath=zetaclient%2Fchains%2Fbitcoin%2Fsigner%2Fsigner.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=zeta-chain#diff-emV0YWNsaWVudC9jaGFpbnMvYml0Y29pbi9zaWduZXIvc2lnbmVyLmdv) | `18.43% <0.00%> (+0.28%)` | :arrow_up: | ... and [5 files with indirect coverage changes](https://app.codecov.io/gh/zeta-chain/node/pull/3184/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=zeta-chain)

gitguardian[bot] commented 12 hours ago

⚠️ GitGuardian has uncovered 1 secret following the scan of your pull request.

Please consider investigating the findings and remediating the incidents. Failure to do so may lead to compromising the associated services or software components.

🔎 Detected hardcoded secret in your pull request

| GitGuardian id | GitGuardian status | Secret | Commit | Filename | | | -------------- | ------------------ | ------------------------------ | ---------------- | --------------- | -------------------- | | [14567965](https://dashboard.gitguardian.com/workspace/353073/incidents/14567965?occurrence=171285380) | Triggered | Generic Password | d723e09b2e7d3e550f4e3c3ace2bfa81ff431851 | cmd/zetaclientd/start.go | [View secret](https://github.com/zeta-chain/node/commit/d723e09b2e7d3e550f4e3c3ace2bfa81ff431851#diff-4f6dad4b36c4f2c7c015aaa31c4468e0fd88293d9fbf4a5f9c1be1751ea5ebe6R161) |

🛠 Guidelines to remediate hardcoded secrets

1. Understand the implications of revoking this secret by investigating where it is used in your code. 2. Replace and store your secret safely. [Learn here](https://blog.gitguardian.com/secrets-api-management) the best practices. 3. Revoke and [rotate this secret](https://docs.gitguardian.com/secrets-detection/detectors/specifics/private_key_openssh#revoke-the-secret). 4. If possible, [rewrite git history](https://blog.gitguardian.com/rewriting-git-history-cheatsheet). Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data. To avoid such incidents in the future consider - following these [best practices](https://blog.gitguardian.com/secrets-api-management/) for managing and storing secrets including API keys and other credentials - install [secret detection on pre-commit](https://https://docs.gitguardian.com/ggshield-docs/integrations/git-hooks/pre-commit) to catch secret before it leaves your machine and ease remediation.

^{🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.}

zeta-chain / node

fix: avoid endless tx rescan when the inbound vote message is invalid #3184