diff --git a/contracts/ccip/ccip/sources/client.move b/contracts/ccip/ccip/sources/client.move index 7d0dc82e..a55989fa 100644 --- a/contracts/ccip/ccip/sources/client.move +++ b/contracts/ccip/ccip/sources/client.move @@ -1,11 +1,13 @@ /// This module defines messages for end users to interact with Aptos CCIP. module ccip::client { use std::bcs; + use std::error; const GENERIC_EXTRA_ARGS_V2_TAG: vector = x"181dcf10"; const SVM_EXTRA_ARGS_V1_TAG: vector = x"1f3b3aba"; const E_INVALID_SVM_TOKEN_RECEIVER_LENGTH: u64 = 1; + const E_INVALID_SVM_ACCOUNT_LENGTH: u64 = 2; #[view] public fun generic_extra_args_v2_tag(): vector { @@ -41,19 +43,35 @@ module ccip::client { extra_args.append(bcs::to_bytes(&compute_units)); extra_args.append(bcs::to_bytes(&account_is_writable_bitmap)); extra_args.append(bcs::to_bytes(&allow_out_of_order_execution)); - if (token_receiver.length() < 32) { - token_receiver.reverse(); - while (token_receiver.length() < 32) { - token_receiver.push_back(0); - }; - token_receiver.reverse(); - }; - assert!(token_receiver.length() == 32, E_INVALID_SVM_TOKEN_RECEIVER_LENGTH); + pad_svm_address(&mut token_receiver); + assert!( + token_receiver.length() == 32, + error::invalid_argument(E_INVALID_SVM_TOKEN_RECEIVER_LENGTH) + ); extra_args.append(bcs::to_bytes(&token_receiver)); + let accounts_len = accounts.length(); + for (i in 0..accounts_len) { + let account = accounts.borrow_mut(i); + pad_svm_address(account); + assert!( + account.length() == 32, + error::invalid_argument(E_INVALID_SVM_ACCOUNT_LENGTH) + ); + }; extra_args.append(bcs::to_bytes(&accounts)); extra_args } + inline fun pad_svm_address(svm_address: &mut vector) { + if (svm_address.length() < 32) { + svm_address.reverse(); + while (svm_address.length() < 32) { + svm_address.push_back(0); + }; + svm_address.reverse(); + } + } + struct Any2AptosMessage has store, drop, copy { message_id: vector, source_chain_selector: u64, @@ -125,4 +143,11 @@ module ccip::client { public fun get_amount(input: &Any2AptosTokenAmount): u64 { input.amount } + + // ======================== Test functions ======================== + + #[test_only] + public fun test_pad_svm_address(svm_address: &mut vector) { + pad_svm_address(svm_address); + } } diff --git a/contracts/ccip/ccip/sources/fee_quoter.move b/contracts/ccip/ccip/sources/fee_quoter.move index 046f5006..71385469 100644 --- a/contracts/ccip/ccip/sources/fee_quoter.move +++ b/contracts/ccip/ccip/sources/fee_quoter.move @@ -246,6 +246,7 @@ module ccip::fee_quoter { const E_INVALID_FEE_RANGE: u64 = 34; const E_INVALID_DEST_BYTES_OVERHEAD: u64 = 35; const E_INVALID_SVM_RECEIVER_LENGTH: u64 = 36; + const E_INVALID_SVM_ACCOUNT_LENGTH: u64 = 37; #[view] public fun type_and_version(): String { @@ -901,6 +902,13 @@ module ccip::fee_quoter { ) * SVM_ACCOUNT_BYTE_SIZE); }; + for (i in 0..accounts_length) { + assert!( + accounts[i].length() == 32, + error::invalid_argument(E_INVALID_SVM_ACCOUNT_LENGTH) + ); + }; + if (tokens_len > 0) { assert!( token_receiver.length() == 32 diff --git a/contracts/ccip/ccip/tests/client_test.move b/contracts/ccip/ccip/tests/client_test.move index db453ad7..9705e1d6 100644 --- a/contracts/ccip/ccip/tests/client_test.move +++ b/contracts/ccip/ccip/tests/client_test.move @@ -156,12 +156,8 @@ module ccip::client_test { } #[test] - #[ - expected_failure( - abort_code = client::E_INVALID_SVM_TOKEN_RECEIVER_LENGTH, - location = ccip::client - ) - ] + #[expected_failure(abort_code = 65537, location = ccip::client)] + // E_INVALID_SVM_TOKEN_RECEIVER_LENGTH fun test_svm_args_rejects_long_token_receiver() { // Test that token receivers longer than 32 bytes are rejected let long_receiver = @@ -169,4 +165,184 @@ module ccip::client_test { // E_INVALID_SVM_TOKEN_RECEIVER_LENGTH client::encode_svm_extra_args_v1(100, 0, false, long_receiver, vector[]); } + + #[test] + fun test_pad_svm_address_empty() { + let addr = vector[]; + client::test_pad_svm_address(&mut addr); + assert!(addr.length() == 32, 0); + // Verify all bytes are zeros + for (i in 0..32) { + assert!(*addr.borrow(i) == 0, 1); + }; + } + + #[test] + fun test_pad_svm_address_single_byte() { + let addr = vector[0x42]; + client::test_pad_svm_address(&mut addr); + assert!(addr.length() == 32, 0); + // First 31 bytes should be zero, last byte should be 0x42 + for (i in 0..31) { + assert!(*addr.borrow(i) == 0, 1); + }; + assert!(*addr.borrow(31) == 0x42, 2); + } + + #[test] + fun test_pad_svm_address_partial() { + let addr = vector[0x01, 0x02, 0x03, 0x04]; + client::test_pad_svm_address(&mut addr); + assert!(addr.length() == 32, 0); + // First 28 bytes should be zero, last 4 bytes should be the original data + for (i in 0..28) { + assert!(*addr.borrow(i) == 0, 1); + }; + assert!(*addr.borrow(28) == 0x01, 2); + assert!(*addr.borrow(29) == 0x02, 3); + assert!(*addr.borrow(30) == 0x03, 4); + assert!(*addr.borrow(31) == 0x04, 5); + } + + #[test] + fun test_pad_svm_address_exact_32_bytes() { + let addr = vector[]; + for (i in 0..32) { + addr.push_back((i as u8)); + }; + let original_addr = addr; + client::test_pad_svm_address(&mut addr); + assert!(addr.length() == 32, 0); + // Should remain unchanged since it's already 32 bytes + assert!(addr == original_addr, 1); + } + + #[test] + fun test_pad_svm_address_31_bytes() { + let addr = vector[]; + for (i in 0..31) { + addr.push_back((i as u8)); + }; + client::test_pad_svm_address(&mut addr); + assert!(addr.length() == 32, 0); + // First byte should be 0 (padding), rest should be the original data + assert!(*addr.borrow(0) == 0, 1); + for (i in 1..32) { + assert!(*addr.borrow(i) == ((i - 1) as u8), 2); + }; + } + + #[test] + fun test_encode_svm_extra_args_v1_basic() { + let token_receiver = vector[0x01, 0x02, 0x03]; + let accounts = vector[vector[0x04, 0x05], vector[0x06, 0x07, 0x08]]; + + let result = + client::encode_svm_extra_args_v1( + 1000u32, 0u64, true, token_receiver, accounts + ); + + // Verify the result starts with the correct tag + let tag_len = client::svm_extra_args_v1_tag().length(); + for (i in 0..tag_len) { + assert!( + *result.borrow(i) == *client::svm_extra_args_v1_tag().borrow(i), + 0 + ); + }; + + // Result should be non-empty and contain the tag + assert!(result.length() > tag_len, 1); + } + + #[test] + fun test_encode_svm_extra_args_v1_empty_accounts() { + let token_receiver = vector[0xFF]; + let accounts = vector[]; + + let result = + client::encode_svm_extra_args_v1( + 500u32, 0u64, false, token_receiver, accounts + ); + + // Should not fail and should contain the tag + let tag_len = client::svm_extra_args_v1_tag().length(); + assert!(result.length() > tag_len, 0); + } + + #[test] + fun test_encode_svm_extra_args_v1_32_byte_addresses() { + let token_receiver = vector[]; + let account1 = vector[]; + let account2 = vector[]; + + // Create exactly 32-byte addresses + for (i in 0..32) { + token_receiver.push_back((i as u8)); + account1.push_back(((i + 100) as u8)); + account2.push_back(((i + 200) as u8)); + }; + + let accounts = vector[account1, account2]; + + let result = + client::encode_svm_extra_args_v1( + 2000u32, + 0xFFFFFFFFFFFFFFFFu64, + true, + token_receiver, + accounts + ); + + // Should succeed without padding since addresses are already 32 bytes + let tag_len = client::svm_extra_args_v1_tag().length(); + assert!(result.length() > tag_len, 0); + } + + #[test] + fun test_encode_svm_extra_args_v1_mixed_address_lengths() { + let token_receiver = vector[0x11]; // 1 byte + let accounts = vector[ + vector[0x22, 0x33], // 2 bytes + vector[], // 0 bytes (empty) + vector[0x44, 0x55, 0x66, 0x77, 0x88] // 5 bytes + ]; + + let result = + client::encode_svm_extra_args_v1( + 750u32, 0u64, false, token_receiver, accounts + ); + + // All addresses should be padded to 32 bytes internally + let tag_len = client::svm_extra_args_v1_tag().length(); + assert!(result.length() > tag_len, 0); + } + + #[test] + #[expected_failure(abort_code = 65537)] + // E_INVALID_SVM_TOKEN_RECEIVER_LENGTH + fun test_encode_svm_extra_args_v1_invalid_token_receiver_length() { + // This test should fail because we're creating a token_receiver that's longer than 32 bytes + let token_receiver = vector[]; + for (i in 0..33) { // 33 bytes - too long + token_receiver.push_back((i as u8)); + }; + let accounts = vector[]; + client::encode_svm_extra_args_v1(1000u32, 0u64, true, token_receiver, accounts); + } + + #[test] + #[expected_failure(abort_code = 65538)] + // E_INVALID_SVM_ACCOUNT_LENGTH + fun test_encode_svm_extra_args_v1_invalid_account_length() { + // This test should fail because we're creating an account that's longer than 32 bytes + let token_receiver = vector[0x01]; + let long_account = vector[]; + for (i in 0..33) { // 33 bytes - too long + long_account.push_back((i as u8)); + }; + let accounts = vector[long_account]; + + client::encode_svm_extra_args_v1(1000u32, 0u64, true, token_receiver, accounts); + } } diff --git a/contracts/ccip/ccip/tests/fee_quoter/fee_quoter_bcs.move b/contracts/ccip/ccip/tests/fee_quoter/fee_quoter_bcs.move index aa6a422a..c33e81af 100644 --- a/contracts/ccip/ccip/tests/fee_quoter/fee_quoter_bcs.move +++ b/contracts/ccip/ccip/tests/fee_quoter/fee_quoter_bcs.move @@ -61,7 +61,13 @@ module ccip::fee_quoter_bcs { assert!(decoded_bitmap == bitmap); assert!(decoded_allow_ooo_svm == allow_ooo_svm); assert!(decoded_token_receiver == token_receiver); - assert!(decoded_accounts == accounts); + + // SVM addresses are padded to 32 bytes during encoding, so decoded accounts will be 32 bytes + let expected_accounts = vector[ + x"0000000000000000000000000000000000000000000000000000000102030405", + x"0000000000000000000000000000000000000000000000000000000504030201" + ]; + assert!(decoded_accounts == expected_accounts); } #[test] @@ -139,17 +145,35 @@ module ccip::fee_quoter_bcs { 100u32, 200u64, false, token_receiver, accounts ); - // Expected size breakdown: - // 4 bytes (SVM_EXTRA_ARGS_V1_TAG) - // 4 bytes (u32 compute_units = 100) - // 8 bytes (u64 bitmap = 200) - // 1 byte (bool allow_ooo = false) - // 33 bytes (BCS-encoded token_receiver: 1 byte length + 32 bytes data) - // 9 bytes (BCS-encoded accounts: 1 byte outer length + 4 bytes first inner + 4 bytes second inner) - // where accounts = vector[vector[1,2,3], vector[4,5,6]] - // = 02 030102030 0304050 6 (in hex) - // Total: 4 + 4 + 8 + 1 + 33 + 9 = 59 bytes - assert!(svm_encoded.length() == 59); // Exact expected size + // Expected size breakdown for BCS encoding: + // + // 1. Fixed-size fields: + // - 4 bytes: SVM_EXTRA_ARGS_V1_TAG + // - 4 bytes: u32 compute_units = 100 + // - 8 bytes: u64 bitmap = 200 + // - 1 byte: bool allow_ooo = false + // Subtotal: 17 bytes + // + // 2. BCS-encoded token_receiver (vector): + // - 1 byte: vector length (32) + // - 32 bytes: actual data (already 32 bytes, no padding needed) + // Subtotal: 33 bytes + // + // 3. BCS-encoded accounts (vector>): + // Original accounts: [vector[1,2,3], vector[4,5,6]] (2 accounts of 3 bytes each) + // After SVM padding: [32-byte account1, 32-byte account2] + // + // BCS encoding structure: + // - 1 byte: outer vector length (2 accounts) + // - For each account (BCS-encoded vector): + // - 1 byte: inner vector length (32) + // - 32 bytes: padded account data + // + // Calculation: 1 + (1 + 32) + (1 + 32) = 1 + 33 + 33 = 67 bytes + // Subtotal: 67 bytes + // + // Total: 17 + 33 + 67 = 117 bytes + assert!(svm_encoded.length() == 117); // Exact expected size with 32-byte SVM addresses } #[test] @@ -297,7 +321,14 @@ module ccip::fee_quoter_bcs { assert!(decoded_bitmap2 == bitmap2, 6); assert!(decoded_ooo2 == allow_ooo2, 7); assert!(decoded_receiver2 == token_receiver2, 8); - assert!(decoded_accounts2 == accounts2, 9); + + // SVM addresses are padded to 32 bytes during encoding + let expected_accounts2 = vector[ + x"0000000000000000000000000000000000000000000000000000000000010203", + x"0000000000000000000000000000000000000000000000000000000000040506", + x"0000000000000000000000000000000000000000000000000000000000070809" + ]; + assert!(decoded_accounts2 == expected_accounts2, 9); // Test case 3: Zero receiver case let compute_units3 = 1000000u32;