-
Notifications
You must be signed in to change notification settings - Fork 2
feat: add SPL token transfer support to build_consolidate #216
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
Changes from all commits
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 | |||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -1050,6 +1050,8 @@ | ||||||||||||||||||||||||||||
| .parse() | |||||||||||||||||||||||||||||
| .map_err(|_| WasmSolanaError::new("Invalid receiveAddress (sender)"))?; | |||||||||||||||||||||||||||||
|
|
|||||||||||||||||||||||||||||
| let default_token_program: Pubkey = SPL_TOKEN_PROGRAM_ID.parse().unwrap(); | |||||||||||||||||||||||||||||
|
|
|||||||||||||||||||||||||||||
| let mut instructions = Vec::new(); | |||||||||||||||||||||||||||||
|
|
|||||||||||||||||||||||||||||
| for recipient in intent.recipients { | |||||||||||||||||||||||||||||
|
|
@@ -1058,19 +1060,73 @@ | ||||||||||||||||||||||||||||
| .as_ref() | |||||||||||||||||||||||||||||
| .map(|a| &a.address) | |||||||||||||||||||||||||||||
| .ok_or_else(|| WasmSolanaError::new("Recipient missing address"))?; | |||||||||||||||||||||||||||||
| let amount = recipient | |||||||||||||||||||||||||||||
| let amount_wrapper = recipient | |||||||||||||||||||||||||||||
| .amount | |||||||||||||||||||||||||||||
| .as_ref() | |||||||||||||||||||||||||||||
| .map(|a| &a.value) | |||||||||||||||||||||||||||||
| .ok_or_else(|| WasmSolanaError::new("Recipient missing amount"))?; | |||||||||||||||||||||||||||||
|
|
|||||||||||||||||||||||||||||
| let to_pubkey: Pubkey = address.parse().map_err(|_| { | |||||||||||||||||||||||||||||
| WasmSolanaError::new(&format!("Invalid recipient address: {}", address)) | |||||||||||||||||||||||||||||
| })?; | |||||||||||||||||||||||||||||
| let lamports: u64 = *amount; | |||||||||||||||||||||||||||||
|
|
|||||||||||||||||||||||||||||
| // Transfer from sender (child address), not fee_payer | |||||||||||||||||||||||||||||
| instructions.push(system_ix::transfer(&sender, &to_pubkey, lamports)); | |||||||||||||||||||||||||||||
| let mint_str = recipient.token_address.as_deref(); | |||||||||||||||||||||||||||||
|
|
|||||||||||||||||||||||||||||
| if let Some(mint_str) = mint_str { | |||||||||||||||||||||||||||||
| // SPL token transfer: sender (child address) is the authority over its own ATA. | |||||||||||||||||||||||||||||
| // The destination ATA is passed in directly by the caller — do NOT re-derive it. | |||||||||||||||||||||||||||||
| let mint: Pubkey = mint_str | |||||||||||||||||||||||||||||
| .parse() | |||||||||||||||||||||||||||||
| .map_err(|_| WasmSolanaError::new(&format!("Invalid token mint: {}", mint_str)))?; | |||||||||||||||||||||||||||||
|
|
|||||||||||||||||||||||||||||
| let token_program: Pubkey = recipient | |||||||||||||||||||||||||||||
| .token_program_id | |||||||||||||||||||||||||||||
| .as_deref() | |||||||||||||||||||||||||||||
| .map(|p| { | |||||||||||||||||||||||||||||
| p.parse() | |||||||||||||||||||||||||||||
| .map_err(|_| WasmSolanaError::new("Invalid tokenProgramId")) | |||||||||||||||||||||||||||||
| }) | |||||||||||||||||||||||||||||
| .transpose()? | |||||||||||||||||||||||||||||
| .unwrap_or(default_token_program); | |||||||||||||||||||||||||||||
|
|
|||||||||||||||||||||||||||||
| let decimals = recipient | |||||||||||||||||||||||||||||
| .decimal_places | |||||||||||||||||||||||||||||
| .ok_or_else(|| WasmSolanaError::new("Token transfer requires decimalPlaces"))?; | |||||||||||||||||||||||||||||
|
|
|||||||||||||||||||||||||||||
| // Source ATA: derive from sender (child address) + mint | |||||||||||||||||||||||||||||
| let sender_ata = derive_ata(&sender, &mint, &token_program); | |||||||||||||||||||||||||||||
|
|
|||||||||||||||||||||||||||||
| // Destination ATA: passed in as-is (already exists on wallet root, caller provides it) | |||||||||||||||||||||||||||||
| let dest_ata = to_pubkey; | |||||||||||||||||||||||||||||
|
|
|||||||||||||||||||||||||||||
| use spl_token::instruction::TokenInstruction; | |||||||||||||||||||||||||||||
| let data = TokenInstruction::TransferChecked { | |||||||||||||||||||||||||||||
| amount: amount_wrapper.value, | |||||||||||||||||||||||||||||
| decimals, | |||||||||||||||||||||||||||||
| } | |||||||||||||||||||||||||||||
| .pack(); | |||||||||||||||||||||||||||||
|
|
|||||||||||||||||||||||||||||
| // Accounts: source(w), mint(r), destination(w), authority(signer) | |||||||||||||||||||||||||||||
| // sender is both signer and owner of the source ATA | |||||||||||||||||||||||||||||
| let transfer_ix = Instruction::new_with_bytes( | |||||||||||||||||||||||||||||
| token_program, | |||||||||||||||||||||||||||||
| &data, | |||||||||||||||||||||||||||||
| vec![ | |||||||||||||||||||||||||||||
| AccountMeta::new(sender_ata, false), | |||||||||||||||||||||||||||||
| AccountMeta::new_readonly(mint, false), | |||||||||||||||||||||||||||||
| AccountMeta::new(dest_ata, false), | |||||||||||||||||||||||||||||
| AccountMeta::new_readonly(sender, true), | |||||||||||||||||||||||||||||
| ], | |||||||||||||||||||||||||||||
| ); | |||||||||||||||||||||||||||||
|
|
|||||||||||||||||||||||||||||
| instructions.push(transfer_ix); | |||||||||||||||||||||||||||||
| } else { | |||||||||||||||||||||||||||||
| // Native SOL transfer from sender (child address), not fee_payer | |||||||||||||||||||||||||||||
| instructions.push(system_ix::transfer( | |||||||||||||||||||||||||||||
| &sender, | |||||||||||||||||||||||||||||
| &to_pubkey, | |||||||||||||||||||||||||||||
| amount_wrapper.value, | |||||||||||||||||||||||||||||
| )); | |||||||||||||||||||||||||||||
| } | |||||||||||||||||||||||||||||
| } | |||||||||||||||||||||||||||||
|
|
|||||||||||||||||||||||||||||
| Ok((instructions, vec![])) | |||||||||||||||||||||||||||||
|
|
@@ -1424,4 +1480,147 @@ | ||||||||||||||||||||||||||||
| "Error should mention missing recipients" | |||||||||||||||||||||||||||||
| ); | |||||||||||||||||||||||||||||
| } | |||||||||||||||||||||||||||||
|
|
|||||||||||||||||||||||||||||
| #[test] | |||||||||||||||||||||||||||||
| fn test_build_consolidate_native_sol() { | |||||||||||||||||||||||||||||
| // Child address consolidates native SOL to root | |||||||||||||||||||||||||||||
| let child_address = "FKjSjCqByQRwSzZoMXA7bKnDbJe41YgJTHFFzBeC42bH"; | |||||||||||||||||||||||||||||
| let root_address = "DgT9qyYwYKBRDyDw3EfR12LHQCQjtNrKu2qMsXHuosmB"; | |||||||||||||||||||||||||||||
|
|
|||||||||||||||||||||||||||||
| let intent = serde_json::json!({ | |||||||||||||||||||||||||||||
| "intentType": "consolidate", | |||||||||||||||||||||||||||||
| "receiveAddress": child_address, | |||||||||||||||||||||||||||||
| "recipients": [{ | |||||||||||||||||||||||||||||
| "address": { "address": root_address }, | |||||||||||||||||||||||||||||
| "amount": { "value": "5000000" } | |||||||||||||||||||||||||||||
| }] | |||||||||||||||||||||||||||||
| }); | |||||||||||||||||||||||||||||
|
|
|||||||||||||||||||||||||||||
| let result = build_from_intent(&intent, &test_params()); | |||||||||||||||||||||||||||||
| assert!(result.is_ok(), "Failed: {:?}", result); | |||||||||||||||||||||||||||||
| let result = result.unwrap(); | |||||||||||||||||||||||||||||
| assert!(result.generated_keypairs.is_empty()); | |||||||||||||||||||||||||||||
|
|
|||||||||||||||||||||||||||||
| // Should have 1 system transfer instruction | |||||||||||||||||||||||||||||
| let msg = result.transaction.message(); | |||||||||||||||||||||||||||||
| assert_eq!( | |||||||||||||||||||||||||||||
| msg.instructions.len(), | |||||||||||||||||||||||||||||
| 1, | |||||||||||||||||||||||||||||
| "Native SOL consolidate should have 1 instruction" | |||||||||||||||||||||||||||||
| ); | |||||||||||||||||||||||||||||
| } | |||||||||||||||||||||||||||||
|
|
|||||||||||||||||||||||||||||
| #[test] | |||||||||||||||||||||||||||||
| fn test_build_consolidate_with_token_recipients() { | |||||||||||||||||||||||||||||
| // Child address consolidates SPL tokens to root's ATA. | |||||||||||||||||||||||||||||
| // The destination ATA is passed in directly by the caller — not re-derived. | |||||||||||||||||||||||||||||
| let child_address = "FKjSjCqByQRwSzZoMXA7bKnDbJe41YgJTHFFzBeC42bH"; | |||||||||||||||||||||||||||||
| // USDC mint | |||||||||||||||||||||||||||||
| let mint = "EPjFWdd5AufqSSqeM2qN1xzybapC8G4wEGGkZwyTDt1v"; | |||||||||||||||||||||||||||||
| // Root wallet's USDC ATA (pre-existing, passed in by caller) | |||||||||||||||||||||||||||||
| let root_ata = "5Q544fKrFoe6tsEbD7S8EmxGTJYAKtTVhAW5Q5pge4j1"; | |||||||||||||||||||||||||||||
|
|
|||||||||||||||||||||||||||||
| let intent = serde_json::json!({ | |||||||||||||||||||||||||||||
| "intentType": "consolidate", | |||||||||||||||||||||||||||||
| "receiveAddress": child_address, | |||||||||||||||||||||||||||||
| "recipients": [{ | |||||||||||||||||||||||||||||
| "address": { "address": root_ata }, | |||||||||||||||||||||||||||||
| "amount": { "value": "1000000" }, | |||||||||||||||||||||||||||||
| "tokenAddress": mint, | |||||||||||||||||||||||||||||
| "decimalPlaces": 6 | |||||||||||||||||||||||||||||
| }] | |||||||||||||||||||||||||||||
| }); | |||||||||||||||||||||||||||||
|
|
|||||||||||||||||||||||||||||
| let result = build_from_intent(&intent, &test_params()); | |||||||||||||||||||||||||||||
| assert!(result.is_ok(), "Failed: {:?}", result); | |||||||||||||||||||||||||||||
Check failureCode scanning / CodeQL Cleartext logging of sensitive information High
This operation writes
... .as_ref() Error loading related location Loading This operation writes ... .as_ref() Error loading related location Loading This operation writes ... .as_ref() Error loading related location Loading This operation writes ...::from_secret_key_bytes(...) Error loading related location Loading
Copilot AutofixAI 3 months ago General approach: Avoid logging potentially sensitive data derived from untrusted or secret sources. In this case, we should stop using Best concrete fix here: Modify the test assertion around line 1535 so it no longer formats What to change exactly: In
Suggested changeset
1
packages/wasm-solana/src/intent/build.rs
Copilot is powered by AI and may make mistakes. Always verify output.
Refresh and try again.
Contributor
Author
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. Same as above — false positive. Hardcoded literal key bytes in a |
|||||||||||||||||||||||||||||
| let result = result.unwrap(); | |||||||||||||||||||||||||||||
| assert!(result.generated_keypairs.is_empty()); | |||||||||||||||||||||||||||||
|
|
|||||||||||||||||||||||||||||
| // Should have 1 transfer_checked instruction (no create ATA — dest already exists) | |||||||||||||||||||||||||||||
| let msg = result.transaction.message(); | |||||||||||||||||||||||||||||
| assert_eq!( | |||||||||||||||||||||||||||||
| msg.instructions.len(), | |||||||||||||||||||||||||||||
| 1, | |||||||||||||||||||||||||||||
| "Token consolidate should have 1 transfer_checked instruction" | |||||||||||||||||||||||||||||
| ); | |||||||||||||||||||||||||||||
|
|
|||||||||||||||||||||||||||||
| // The instruction should use the SPL Token program | |||||||||||||||||||||||||||||
| let token_program: Pubkey = SPL_TOKEN_PROGRAM_ID.parse().unwrap(); | |||||||||||||||||||||||||||||
| let ix = &msg.instructions[0]; | |||||||||||||||||||||||||||||
| let program_id = msg.account_keys[ix.program_id_index as usize]; | |||||||||||||||||||||||||||||
| assert_eq!( | |||||||||||||||||||||||||||||
| program_id, token_program, | |||||||||||||||||||||||||||||
| "Instruction should use SPL Token program" | |||||||||||||||||||||||||||||
| ); | |||||||||||||||||||||||||||||
|
|
|||||||||||||||||||||||||||||
| // Verify account count: source ATA, mint, dest ATA, authority (sender/child) = 4 | |||||||||||||||||||||||||||||
| assert_eq!( | |||||||||||||||||||||||||||||
| ix.accounts.len(), | |||||||||||||||||||||||||||||
| 4, | |||||||||||||||||||||||||||||
| "transfer_checked should have 4 accounts" | |||||||||||||||||||||||||||||
| ); | |||||||||||||||||||||||||||||
| } | |||||||||||||||||||||||||||||
|
|
|||||||||||||||||||||||||||||
| #[test] | |||||||||||||||||||||||||||||
| fn test_build_consolidate_token_missing_decimal_places() { | |||||||||||||||||||||||||||||
| let child_address = "FKjSjCqByQRwSzZoMXA7bKnDbJe41YgJTHFFzBeC42bH"; | |||||||||||||||||||||||||||||
| let mint = "EPjFWdd5AufqSSqeM2qN1xzybapC8G4wEGGkZwyTDt1v"; | |||||||||||||||||||||||||||||
| let root_ata = "5Q544fKrFoe6tsEbD7S8EmxGTJYAKtTVhAW5Q5pge4j1"; | |||||||||||||||||||||||||||||
|
|
|||||||||||||||||||||||||||||
| let intent = serde_json::json!({ | |||||||||||||||||||||||||||||
| "intentType": "consolidate", | |||||||||||||||||||||||||||||
| "receiveAddress": child_address, | |||||||||||||||||||||||||||||
| "recipients": [{ | |||||||||||||||||||||||||||||
| "address": { "address": root_ata }, | |||||||||||||||||||||||||||||
| "amount": { "value": "1000000" }, | |||||||||||||||||||||||||||||
| "tokenAddress": mint | |||||||||||||||||||||||||||||
| // decimalPlaces intentionally omitted | |||||||||||||||||||||||||||||
| }] | |||||||||||||||||||||||||||||
| }); | |||||||||||||||||||||||||||||
|
|
|||||||||||||||||||||||||||||
| let result = build_from_intent(&intent, &test_params()); | |||||||||||||||||||||||||||||
| assert!(result.is_err(), "Should fail without decimalPlaces"); | |||||||||||||||||||||||||||||
| assert!( | |||||||||||||||||||||||||||||
| result.unwrap_err().to_string().contains("decimalPlaces"), | |||||||||||||||||||||||||||||
| "Error should mention decimalPlaces" | |||||||||||||||||||||||||||||
| ); | |||||||||||||||||||||||||||||
| } | |||||||||||||||||||||||||||||
|
|
|||||||||||||||||||||||||||||
| #[test] | |||||||||||||||||||||||||||||
| fn test_build_consolidate_mixed_recipients() { | |||||||||||||||||||||||||||||
| // One native SOL recipient and one token recipient | |||||||||||||||||||||||||||||
| let child_address = "FKjSjCqByQRwSzZoMXA7bKnDbJe41YgJTHFFzBeC42bH"; | |||||||||||||||||||||||||||||
| let root_address = "DgT9qyYwYKBRDyDw3EfR12LHQCQjtNrKu2qMsXHuosmB"; | |||||||||||||||||||||||||||||
| let mint = "EPjFWdd5AufqSSqeM2qN1xzybapC8G4wEGGkZwyTDt1v"; | |||||||||||||||||||||||||||||
| let root_ata = "5Q544fKrFoe6tsEbD7S8EmxGTJYAKtTVhAW5Q5pge4j1"; | |||||||||||||||||||||||||||||
|
|
|||||||||||||||||||||||||||||
| let intent = serde_json::json!({ | |||||||||||||||||||||||||||||
| "intentType": "consolidate", | |||||||||||||||||||||||||||||
| "receiveAddress": child_address, | |||||||||||||||||||||||||||||
| "recipients": [ | |||||||||||||||||||||||||||||
| { | |||||||||||||||||||||||||||||
| "address": { "address": root_address }, | |||||||||||||||||||||||||||||
| "amount": { "value": "5000000" } | |||||||||||||||||||||||||||||
| }, | |||||||||||||||||||||||||||||
| { | |||||||||||||||||||||||||||||
| "address": { "address": root_ata }, | |||||||||||||||||||||||||||||
| "amount": { "value": "2000000" }, | |||||||||||||||||||||||||||||
| "tokenAddress": mint, | |||||||||||||||||||||||||||||
| "decimalPlaces": 6 | |||||||||||||||||||||||||||||
| } | |||||||||||||||||||||||||||||
| ] | |||||||||||||||||||||||||||||
| }); | |||||||||||||||||||||||||||||
|
|
|||||||||||||||||||||||||||||
| let result = build_from_intent(&intent, &test_params()); | |||||||||||||||||||||||||||||
| assert!(result.is_ok(), "Failed: {:?}", result); | |||||||||||||||||||||||||||||
Check failureCode scanning / CodeQL Cleartext logging of sensitive information High
This operation writes
... .as_ref() Error loading related location Loading This operation writes ... .as_ref() Error loading related location Loading This operation writes ... .as_ref() Error loading related location Loading This operation writes ...::from_secret_key_bytes(...) Error loading related location Loading
Copilot AutofixAI 3 months ago In general, to fix cleartext logging of sensitive information, avoid formatting or logging values that may contain secrets (keys, tokens, addresses, configs). Instead, log only high-level status or sanitized identifiers, or remove the log entirely if it isn’t necessary. Here, the only flagged sink is the assert!(result.is_ok(), "Failed: {:?}", result);The Concretely:
This single change removes all flows from secret/untrusted data into the log-formatted string while keeping the test semantics (it still fails if
Suggested changeset
1
packages/wasm-solana/src/intent/build.rs
Copilot is powered by AI and may make mistakes. Always verify output.
Refresh and try again.
Contributor
Author
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. Same as above — false positive. Hardcoded literal key bytes in a |
|||||||||||||||||||||||||||||
| let result = result.unwrap(); | |||||||||||||||||||||||||||||
|
|
|||||||||||||||||||||||||||||
| // Should have 2 instructions: 1 system transfer + 1 transfer_checked | |||||||||||||||||||||||||||||
| let msg = result.transaction.message(); | |||||||||||||||||||||||||||||
| assert_eq!( | |||||||||||||||||||||||||||||
| msg.instructions.len(), | |||||||||||||||||||||||||||||
| 2, | |||||||||||||||||||||||||||||
| "Mixed consolidate should have 2 instructions" | |||||||||||||||||||||||||||||
| ); | |||||||||||||||||||||||||||||
| } | |||||||||||||||||||||||||||||
| } | |||||||||||||||||||||||||||||
Check failure
Code scanning / CodeQL
Cleartext logging of sensitive information High
Copilot Autofix
AI 3 months ago
In general, to fix cleartext logging/leakage issues, you should avoid including sensitive or tainted data in log or panic messages, especially with broad
Debugformatting (like"{:?}") on large composite structures. When diagnostic information is needed, either omit the sensitive fields or replace them with redacted/summary information that cannot be misused.For this specific case, the problematic sink is the assertion:
Here,
resultis tainted, andassert!will format it with"{:?}"if the assertion fails, potentially outputting sensitive values. The simplest way to eliminate the leak without changing functionality is:resultin the panic message.Concretely, change line 1500 to a version that does not format
result, e.g.:This keeps the test’s behavior—failing when
resultis notOk—but prevents any tainted data from being written to stderr/logs. No additional imports or helpers are needed. This single change should address all alert variants, because they all share the same sink (assert!with"{:?}", result); onceresultis no longer formatted, the tainted data paths into that formatting operation are removed.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.
False positive — these are hardcoded test keypairs used only in unit tests to exercise the build_consolidate instruction logic. The key bytes never come from user input or any real secret store. The pattern flagged (
from_secret_key_bytes) is the standard way to construct a deterministicKeypairin Solana test code; the bytes are literals baked into the source. Nothing is written to any log file in production — this is dead code outside of#[cfg(test)]blocks.