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.

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

Feat/split receiver #448

Merged
merged 21 commits into from
Jan 21, 2025
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -408,7 +408,7 @@ fn internal_execute<'info>(
calculate_token_pool_account_indices(i, token_indexes, ctx.remaining_accounts.len())?;
let acc_list = &ctx.remaining_accounts[start..end];
let accs = validate_and_parse_token_accounts(
execution_report.message.receiver,
execution_report.message.token_receiver,
execution_report.message.header.source_chain_selector,
ctx.program_id.key(),
acc_list,
Expand All @@ -420,7 +420,7 @@ fn internal_execute<'info>(
// CPI: call lockOrBurn on token pool
let release_or_mint = ReleaseOrMintInV1 {
original_sender: execution_report.message.sender.clone(),
receiver: execution_report.message.receiver,
receiver: execution_report.message.token_receiver,
amount: token_amount.amount,
local_token: token_amount.dest_token_address,
remote_chain_selector: execution_report.message.header.source_chain_selector,
Expand Down Expand Up @@ -475,10 +475,13 @@ fn internal_execute<'info>(
// handle CPI call if there are extra accounts
// case: no tokens, but there are remaining_accounts passed in
// case: tokens, but the first token has a non-zero index (indicating extra accounts before token accounts)
if should_execute_messaging(token_indexes, ctx.remaining_accounts.is_empty()) {
if should_execute_messaging(
&execution_report.message.logic_receiver,
ctx.remaining_accounts.is_empty(),
) {
let (msg_program, msg_accounts) = parse_messaging_accounts(
token_indexes,
execution_report.message.receiver,
execution_report.message.logic_receiver,
&execution_report.message.extra_args.accounts,
&execution_report.message.extra_args.is_writable_bitmap,
ctx.remaining_accounts,
Expand Down Expand Up @@ -538,72 +541,59 @@ fn internal_execute<'info>(
Ok(())
}

// should_execute_messaging checks if there remaining_accounts that are not being used for token pools
// case: no tokens, but there are remaining_accounts passed in
// case: tokens, but the first token has a non-zero index (indicating extra accounts before token accounts)
fn should_execute_messaging(token_indexes: &[u8], remaining_accounts_empty: bool) -> bool {
(token_indexes.is_empty() && !remaining_accounts_empty)
|| (!token_indexes.is_empty() && token_indexes[0] != 0)
// should_execute_messaging checks if:
// 1. There is at least one account used for messaging (the first subset of accounts). This is because the first account is the program id to do the CPI
// 2. AND the logic_receiver has a value different than zeros
fn should_execute_messaging(logic_receiver: &Pubkey, remaining_accounts_empty: bool) -> bool {
!remaining_accounts_empty && *logic_receiver != Pubkey::default()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should we check here for the compute Units as well?

}

/// parse_message_accounts returns all the accounts needed to execute the CPI instruction
/// It also validates that the accounts sent in the message match the ones sent in the source chain
/// Precondition: logic_receiver != 0 && remaining_accounts.len() > 0
///
/// # Arguments
/// * `token_indexes` - start indexes of token pool accounts, used to determine ending index for arbitrary messaging accounts
/// * `receiver` - receiver address from x-chain message, used to validate `accounts`
/// * `source_accounts` - arbitrary messaging accounts from the x-chain message, used to validate `accounts`. expected order is: [program, ...additional message accounts]
/// * `accounts` - accounts passed via `ctx.remaining_accounts`. expected order is: [program, receiver, ...additional message accounts]
/// * `logic_receiver` - receiver address from x-chain message, used to validate `remaining_accounts`
/// * `extra_args_accounts` - arbitrary messaging accounts from the x-chain message, used to validate `accounts`.
/// * `remaining_accounts` - accounts passed via `ctx.remaining_accounts`. expected order is: [program, receiver, ...additional message accounts]
fn parse_messaging_accounts<'info>(
token_indexes: &[u8],
receiver: Pubkey,
source_accounts: &[Pubkey],
logic_receiver: Pubkey,
extra_args_accounts: &[Pubkey],
Copy link
Contributor

Choose a reason for hiding this comment

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

I got rid of all of these here, just as a heads up to make sure what we've done works well together (I don't see why not: I don't mind rebasing on you if this gets merged first)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This change is aming for clarity on the users and the protocol itself, it doesn't add any functionality. We need to establish an abstraction that is super clear for the users/offchain code what to send, and the receiver (token or logic) is the most sensitive part of the protocol, we can't have any missunderstanding or wrong configuration. We could only receive accounts and do all the validations, but this way we are sure that all the nodes have signed those addresses as logic and token receivers.

source_bitmap: &u64,
accounts: &'info [AccountInfo<'info>],
remaining_accounts: &'info [AccountInfo<'info>],
) -> Result<(&'info AccountInfo<'info>, &'info [AccountInfo<'info>])> {
let end_ind = if token_indexes.is_empty() {
accounts.len()
let end_index = if token_indexes.is_empty() {
remaining_accounts.len()
} else {
token_indexes[0] as usize
};

let msg_program = &accounts[0];
let msg_accounts = &accounts[1..end_ind];
require!(
1 <= end_index && end_index <= remaining_accounts.len(), // program id and message accounts need to fit in remaining accounts
CcipRouterError::InvalidInputs
); // there could be other remaining accounts used for tokens

let source_program = &source_accounts[0];
let source_msg_accounts = &source_accounts[1..source_accounts.len()];
let msg_program = &remaining_accounts[0];
let msg_accounts = &remaining_accounts[1..end_index];

require!(
*source_program == msg_program.key(),
logic_receiver == msg_program.key(),
CcipRouterError::InvalidInputs,
);

require!(
msg_accounts[0].key() == receiver,
CcipRouterError::InvalidInputs
);

// assert same number of accounts passed from message and transaction (not including program)
// source_msg_accounts + 1 to account for separately passed receiver address
require!(
source_msg_accounts.len() + 1 == msg_accounts.len(),
msg_accounts.len() == extra_args_accounts.len(), // assert same number of accounts passed from message and transaction
CcipRouterError::InvalidInputs
);

// Validate the addresses of all the accounts match the ones in source chain
if msg_accounts.len() > 1 {
// Ignore the first account as it's the receiver
let accounts_to_validate = &msg_accounts[1..msg_accounts.len()];
require!(
accounts_to_validate.len() == source_msg_accounts.len(),
CcipRouterError::InvalidInputs
);
for (i, acc) in source_msg_accounts.iter().enumerate() {
let current_acc = &msg_accounts[i + 1]; // TODO: remove offset by 1 to skip receiver after receiver refactor
for (i, acc) in extra_args_accounts.iter().enumerate() {
let current_acc = &msg_accounts[i];
require!(*acc == current_acc.key(), CcipRouterError::InvalidInputs);
require!(
// TODO: remove offset by 1 to skip program after receiver refactor
is_writable(source_bitmap, (i + 1) as u8) == current_acc.is_writable,
is_writable(source_bitmap, (i) as u8) == current_acc.is_writable,
CcipRouterError::InvalidInputs
);
}
Expand Down Expand Up @@ -636,7 +626,6 @@ pub fn verify_merkle_root(execution_report: &ExecutionReportSingleChain) -> Resu
Ok(hashed_leaf)
}

// TODO: Refactor this to use the same structure as messages: execution_report.validate(..)
pub fn validate_execution_report<'info>(
execution_report: &ExecutionReportSingleChain,
source_chain_state: &Account<'info, SourceChain>,
Expand Down Expand Up @@ -702,7 +691,8 @@ fn hash(msg: &Any2SVMRampMessage) -> [u8; 32] {
&msg.on_ramp_address,
// message header
&msg.header.message_id,
&msg.receiver.to_bytes(),
&msg.token_receiver.to_bytes(),
&msg.logic_receiver.to_bytes(),
&header_sequence_number,
msg.extra_args.try_to_vec().unwrap().as_ref(), // borsh serialized
&header_nonce,
Expand Down Expand Up @@ -852,7 +842,10 @@ mod tests {
0, 0, 0, 0,
]
.to_vec(),
receiver: Pubkey::try_from("DS2tt4BX7YwCw7yrDNwbAdnYrxjeCPeGJbHmZEYC8RTb").unwrap(),
token_receiver: Pubkey::try_from("DS2tt4BX7YwCw7yrDNwbAdnYrxjeCPeGJbHmZEYC8RTb")
.unwrap(),
logic_receiver: Pubkey::try_from("C8WSPj3yyus1YN3yNB6YA5zStYtbjQWtpmKadmvyUXq8")
.unwrap(),
data: vec![4, 5, 6],
header: RampMessageHeader {
message_id: [
Expand All @@ -879,15 +872,15 @@ mod tests {
compute_units: 1000,
is_writable_bitmap: 1,
accounts: vec![
Pubkey::try_from("DS2tt4BX7YwCw7yrDNwbAdnYrxjeCPeGJbHmZEYC8RTb").unwrap(),
Pubkey::try_from("CtEVnHsQzhTNWav8skikiV2oF6Xx7r7uGGa8eCDQtTjH").unwrap(),
],
},
on_ramp_address: on_ramp_address.clone(),
};
let hash_result = hash(&message);

assert_eq!(
"60f412fe7c28ae6981b694f92677276f767a98e0314b9a31a3c38366223e7e52",
"266b8d99e64a52fdd325f67674f56d0005dbee5e9999ff22017d5b117fbedfa3",
hex::encode(hash_result)
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ pub struct TokenAccounts<'a> {
}

pub fn validate_and_parse_token_accounts<'info>(
user: Pubkey,
token_receiver: Pubkey,
chain_selector: u64,
router: Pubkey,
accounts: &'info [AccountInfo<'info>],
Expand Down Expand Up @@ -122,7 +122,7 @@ pub fn validate_and_parse_token_accounts<'info>(
require!(
user_token_account.key()
== get_associated_token_address_with_program_id(
&user,
&token_receiver,
&mint.key(),
&token_program.key()
)
Expand Down
14 changes: 9 additions & 5 deletions chains/solana/contracts/programs/ccip-router/src/messages.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,10 +59,13 @@ pub struct Any2SVMRampMessage {
pub header: RampMessageHeader,
pub sender: Vec<u8>,
pub data: Vec<u8>,
// receiver is used as the target for the two main functionalities
// token transfers: recipient of token transfers (associated token addresses are validated against this address)
// arbitrary messaging: expected account in the declared arbitrary messaging accounts (2nd in the list of the accounts)
pub receiver: Pubkey,
// In EVM receiver means the address that all the listed tokens will transfer to and the address of the message execution.
// In Solana the receiver is split into two:
// Logic Receiver is the Program ID of the user's program that will execute the message
pub logic_receiver: Pubkey,
Copy link
Contributor

Choose a reason for hiding this comment

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

Wdyt about naming this message_receiver instead of logic_receiver? It feels closer to CCIP terminology in general, e.g. ccip use cases include "pure messaging" (thus message_receiver) vs "token transfer" vs "programmable token transfer" (which is the sum of the previous two)

Alternatively, using more of the Solana terminology, we could name it cpi_receiver

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is actually no term for CCIP to distinguish the receiver between logic and tokens as for EVM they are both the same.

Naming them logic_receiver and token_receiver makes it impossible to have ambiguity between the two terms. Someone could argue that any CPI call is a message receiver or cpi receiver, those are more generics names.

// Token Receiver is the address which the ATA will be calculated from.
// If token receiver and message execution, then the token receiver must be a PDA from the logic receiver
pub token_receiver: Pubkey,
pub token_amounts: Vec<Any2SVMTokenTransfer>,
pub extra_args: SVMExtraArgs,
pub on_ramp_address: Vec<u8>,
Expand All @@ -75,7 +78,8 @@ impl Any2SVMRampMessage {
self.header.len() // header
+ 4 + self.sender.len() // sender
+ 4 + self.data.len() // data
+ 32 // receiver
+ 32 // logic receiver
+ 32 // token receiver
+ 4 + token_len // token_amount
+ self.extra_args.len() // extra_args
+ 4 + self.on_ramp_address.len() // on_ramp_address
Expand Down
6 changes: 5 additions & 1 deletion chains/solana/contracts/target/idl/ccip_router.json
Original file line number Diff line number Diff line change
Expand Up @@ -2187,7 +2187,11 @@
"type": "bytes"
},
{
"name": "receiver",
"name": "logicReceiver",
"type": "publicKey"
},
{
"name": "tokenReceiver",
"type": "publicKey"
},
{
Expand Down
Loading
Loading