Solana CCTP miscellanous fixes #1102
Merged
+233
−142
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…fixes of re-generating code
|
👋 toblich, thanks for creating this pull request! To help reviewers, please consider creating future PRs as drafts first. This allows you to self-review and make any final changes before notifying the team. Once you're ready, you can mark it as "Ready for review" to request feedback. Thanks! |
There was a problem hiding this comment.
Pull Request Overview
This PR contains miscellaneous fixes for Solana CCTP implementation, focusing on improving clarity, correctness, and consistency across the codebase. The changes include documentation improvements, spelling corrections, error message fixes, code simplification, and parameter adjustments.
- Documentation and comment clarity improvements for CCTP message transmission relationships
- Spelling and grammatical corrections in error messages and comments
- Code simplification and API consistency improvements for function parameters and return types
- Removal of unused code and better validation of ownership transfer operations
Reviewed Changes
Copilot reviewed 24 out of 24 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| chains/solana/gobindings/latest/cctp_token_pool/LockOrBurnTokens.go | Updated documentation to clarify CCTP invocation relationships |
| chains/solana/gobindings/latest/ccip_offramp/types.go | Renamed interface and types for better naming consistency |
| chains/solana/contracts/target/types/cctp_token_pool.ts | Updated TypeScript type definitions to match Go binding changes |
| chains/solana/contracts/target/types/ccip_offramp.ts | Updated TypeScript type definitions for renamed types |
| chains/solana/contracts/target/types/base_token_pool.ts | Fixed spelling error in error message |
| chains/solana/contracts/target/idl/*.json | Updated IDL files to reflect type and message changes |
| chains/solana/contracts/programs/rmn-remote/src/instructions/v1/admin.rs | Added validation to prevent setting owner to default public key |
| chains/solana/contracts/programs/fee-quoter/src/instructions/v1/admin.rs | Added validation to prevent setting owner to default public key |
| chains/solana/contracts/programs/example-ccip-sender/src/lib.rs | Removed unnecessary cfg attribute |
| chains/solana/contracts/programs/example-ccip-sender/Cargo.toml | Added proper lint configuration for cfg warnings |
| chains/solana/contracts/programs/cctp-token-pool/src/lib.rs | Fixed typo in comment |
| chains/solana/contracts/programs/cctp-token-pool/src/context.rs | Updated documentation and reorganized validation order |
| chains/solana/contracts/programs/ccip-router/src/state.rs | Removed unused method from DeriveAccountsResponse |
| chains/solana/contracts/programs/ccip-router/src/instructions/v1/admin.rs | Added validation to prevent setting owner to default public key |
| chains/solana/contracts/programs/ccip-offramp/src/state.rs | Removed unused method from DeriveAccountsResponse |
| chains/solana/contracts/programs/ccip-offramp/src/instructions/v1/execute/derive.rs | Renamed enum and updated all references |
| chains/solana/contracts/programs/ccip-offramp/src/instructions/v1/execute.rs | Updated to use renamed enum and simplified iterator usage |
| chains/solana/contracts/programs/ccip-offramp/src/instructions/v1/admin.rs | Added validation to prevent setting owner to default public key |
| chains/solana/contracts/programs/ccip-offramp/src/context.rs | Removed unused parameters from instruction contexts |
| chains/solana/contracts/programs/ccip-common/src/v1.rs | Refactored token admin registry loading for better maintainability |
| chains/solana/contracts/programs/ccip-common/src/lib.rs | Added new struct and conversion for token admin registry v1 |
| chains/solana/contracts/programs/base-token-pool/src/common.rs | Fixed spelling errors in error messages and comments |
chains/solana/contracts/programs/ccip-offramp/src/instructions/v1/execute.rs
Show resolved
Hide resolved
| @@ -272,29 +275,15 @@ fn load_v1_token_admin_registry( | |||
| ); | |||
|
|
|||
| require!( | |||
| TokenAdminRegistry::DISCRIMINATOR == data[..8], | |||
| TokenAdminRegistry::DISCRIMINATOR == data[..ANCHOR_DISCRIMINATOR_SIZE], | |||
There was a problem hiding this comment.
This is just a nit, and one that there's no need to act on now as it's pervasive on the codebase, but for situations like this in the future I think it's a bit cleaner to use split_at rather than using slice ranges.
| mint: Pubkey::new_from_array(data[137..169].try_into().unwrap()), | ||
| supports_auto_derivation: false, // this is the field that is missing in v1, and it defaults to false | ||
| }) | ||
| Ok(TokenAdminRegistry::try_from(token_admin_registry_v1)?) |
| require_keys_neq!( | ||
| proposed_owner, | ||
| Pubkey::default(), | ||
| CcipRouterError::RedundantOwnerProposal |
There was a problem hiding this comment.
This error message is incorrect now. It doesn't seem like a big deal here, but the error string reads "Proposed owner is the current owner", which could be confusing in this context.
|
PabloMansanet
approved these changes
Jul 28, 2025
agusaldasoro
approved these changes
Jul 28, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Suggestion for reviewers: Look at the diff commit-by-commit, as they are all fairly atomic