Skip to content
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.

Sign up for GitHub

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

solana: validate evm extra args #481

Merged
merged 3 commits into from
Jan 22, 2025
Merged

solana: validate evm extra args #481

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6ff89e0
solana: validate evm extra args
aalu1418 Jan 21, 2025
4bac688
Merge branch 'main' into solana/validate-extra-args
aalu1418 Jan 22, 2025
bc669c1
Merge branch 'main' into solana/validate-extra-args
aalu1418 Jan 22, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions chains/solana/contracts/programs/ccip-router/src/instructions/v1/messages.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,20 @@ pub mod ramps {
CcipRouterError::UnsupportedNumberOfTokens
);

require_gte!(
dest_chain.config.max_per_msg_gas_limit as u128,
msg.extra_args
.gas_limit
.unwrap_or(dest_chain.config.default_tx_gas_limit as u128),
CcipRouterError::MessageGasLimitTooHigh,
);

require!(
!dest_chain.config.enforce_out_of_order
|| msg.extra_args.allow_out_of_order_execution.unwrap_or(false),
CcipRouterError::ExtraArgOutOfOrderExecutionMustBeTrue,
);

validate_dest_family_address(msg, dest_chain.config.chain_family_selector)
}

Expand Down
4 changes: 4 additions & 0 deletions chains/solana/contracts/programs/ccip-router/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -683,4 +683,8 @@ pub enum CcipRouterError {
MessageFeeTooHigh,
#[msg("Source token data is too large")]
SourceTokenDataTooLarge,
#[msg("Message gas limit too high")]
MessageGasLimitTooHigh,
#[msg("Extra arg out of order execution must be true")]
ExtraArgOutOfOrderExecutionMustBeTrue,
}
6 changes: 6 additions & 0 deletions chains/solana/contracts/target/idl/ccip_router.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2773,6 +2773,12 @@
},
{
"name": "SourceTokenDataTooLarge"
},
{
"name": "MessageGasLimitTooHigh"
},
{
"name": "ExtraArgOutOfOrderExecutionMustBeTrue"
}
]
}
Expand Down
79 changes: 78 additions & 1 deletion chains/solana/contracts/tests/ccip/ccip_router_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ func TestCCIPRouter(t *testing.T) {
require.NoError(t, gerr)

var nonceEvmPDA solana.PublicKey
var nonceSvmPDA solana.PublicKey

// billing
type AccountsPerToken struct {
Expand Down Expand Up @@ -393,6 +394,8 @@ func TestCCIPRouter(t *testing.T) {

nonceEvmPDA, err = ccip.GetNoncePDA(config.EvmChainSelector, user.PublicKey())
require.NoError(t, err)
nonceSvmPDA, err = ccip.GetNoncePDA(config.SVMChainSelector, user.PublicKey())
require.NoError(t, err)
})

t.Run("When admin updates the default gas limit it's updated", func(t *testing.T) {
Expand Down Expand Up @@ -586,7 +589,9 @@ func TestCCIPRouter(t *testing.T) {
// minimal valid config
DefaultTxGasLimit: 1,
MaxPerMsgGasLimit: 100,
ChainFamilySelector: [4]uint8{3, 2, 1, 0}},
ChainFamilySelector: [4]uint8{3, 2, 1, 0},
EnforceOutOfOrder: true,
},
config.SVMSourceChainStatePDA,
config.SVMDestChainStatePDA,
config.RouterConfigPDA,
Expand Down Expand Up @@ -2374,6 +2379,78 @@ func TestCCIPRouter(t *testing.T) {
require.Equal(t, uint64(3), ccipMessageSentEvent.Message.Header.Nonce)
})

t.Run("When gasLimit is too high, it fails", func(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Echoing @toblich's comment on a PR of mine a while ago, this kind of validation can be captured with a Rust unit test rather than the comparatively heavier Go integration test, which I think will help us with iteration time (it's easy to run a single Rust test/group of tests, while the Go suite is more coupled and order-dependent).

I don't have a strong opinion and I wouldn't stop the merge on this, but I'd mildly prefer this to be a Rust unit test instead :).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ah good point - i will need to do a bit of refactoring here anyway to support multiple types so I will move the tests in the next PR!

destinationChainSelector := config.EvmChainSelector
destinationChainStatePDA := config.EvmDestChainStatePDA
message := ccip_router.SVM2AnyMessage{
FeeToken: token2022.mint,
Receiver: validReceiverAddress[:],
Data: []byte{4, 5, 6},
ExtraArgs: ccip_router.ExtraArgsInput{
GasLimit: &bin.Uint128{Lo: 0, Hi: 1_000_000_000},
},
}

raw := ccip_router.NewCcipSendInstruction(
destinationChainSelector,
message,
[]byte{},
config.RouterConfigPDA,
destinationChainStatePDA,
nonceEvmPDA,
user.PublicKey(),
solana.SystemProgramID,
token2022.program,
token2022.mint,
token2022.billingConfigPDA,
token2022.billingConfigPDA,
token2022.userATA,
token2022.billingATA,
config.BillingSignerPDA,
config.ExternalTokenPoolsSignerPDA,
)
raw.GetFeeTokenUserAssociatedAccountAccount().WRITE()
instruction, err := raw.ValidateAndBuild()
require.NoError(t, err)
testutils.SendAndFailWith(ctx, t, solanaGoClient, []solana.Instruction{instruction}, user, config.DefaultCommitment, []string{ccip_router.MessageGasLimitTooHigh_CcipRouterError.String()})
})

t.Run("When out of order execution is enforced, it fails when not enabled", func(t *testing.T) {
destinationChainSelector := config.SVMChainSelector // SVM dest chain requires out of order execution
destinationChainStatePDA := config.SVMDestChainStatePDA
falseVal := false
message := ccip_router.SVM2AnyMessage{
FeeToken: token2022.mint,
Receiver: validReceiverAddress[:],
ExtraArgs: ccip_router.ExtraArgsInput{
AllowOutOfOrderExecution: &falseVal,
},
}

raw := ccip_router.NewCcipSendInstruction(
destinationChainSelector,
message,
[]byte{},
config.RouterConfigPDA,
destinationChainStatePDA,
nonceSvmPDA,
user.PublicKey(),
solana.SystemProgramID,
token2022.program,
token2022.mint,
token2022.billingConfigPDA,
token2022.billingConfigPDA,
token2022.userATA,
token2022.billingATA,
config.BillingSignerPDA,
config.ExternalTokenPoolsSignerPDA,
)
raw.GetFeeTokenUserAssociatedAccountAccount().WRITE()
instruction, err := raw.ValidateAndBuild()
require.NoError(t, err)
testutils.SendAndFailWith(ctx, t, solanaGoClient, []solana.Instruction{instruction}, user, config.DefaultCommitment, []string{ccip_router.ExtraArgOutOfOrderExecutionMustBeTrue_CcipRouterError.String()})
})

t.Run("When sending a message with an invalid nonce account, it fails", func(t *testing.T) {
destinationChainSelector := config.EvmChainSelector
destinationChainStatePDA := config.EvmDestChainStatePDA
Expand Down
6 changes: 6 additions & 0 deletions chains/solana/gobindings/ccip_router/types.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading