-
Notifications
You must be signed in to change notification settings - Fork 7
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
Feat/split receiver #448
Changes from 13 commits
956e036
ad61baa
a8af56c
2938a0c
8829ddf
10f6083
68a4bf1
73c3cd6
2553dd9
2071c11
11f1fc2
f4f9365
754a41e
2efed9a
8ecb128
cd87b13
24913cf
862bea2
ebe4f3c
95aa18a
25b50e7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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, | ||
|
@@ -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, | ||
|
@@ -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, | ||
|
@@ -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() | ||
} | ||
|
||
/// 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], | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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) There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||
); | ||
} | ||
|
@@ -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>, | ||
|
@@ -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, | ||
|
@@ -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: [ | ||
|
@@ -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) | ||
); | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wdyt about naming this Alternatively, using more of the Solana terminology, we could name it There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
// 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>, | ||
|
@@ -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 | ||
|
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.
Should we check here for the compute Units as well?