From 04e925fe81a585f60bc4fbd9caf7d31e8e7422ef Mon Sep 17 00:00:00 2001 From: Diana Carvalho Date: Thu, 23 Jan 2025 11:21:06 +0000 Subject: [PATCH] fix: Correct encoding of the approvals - Add data validations similar to the ones done in the Permit2 SDK - Fix Domain main (!!!) It's Permit2 not Permit - Return the whole function signature data (owner, permit_single, signature) encoded test improvements: - Don't compare the timestamps, this was making the test fail sometimes and pass other times - Add a test to run on an anvil fork and actually call permit2 contract to double check the encoded data works misc: Rename get_allowance_data -> get_existing_allowance --- don't change below this line --- ENG-4063 Took 5 hours 19 minutes Took 11 seconds --- Cargo.toml | 1 + src/encoding/evm/approvals/permit2.rs | 163 ++++++++++++++++++++++---- 2 files changed, 143 insertions(+), 21 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index fd77947..0f764c5 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -26,6 +26,7 @@ rstest = "0.24.0" [features] default = ["evm"] evm = ["alloy", "alloy-sol-types", "alloy-primitives"] +fork-tests = [] [profile.bench] debug = true diff --git a/src/encoding/evm/approvals/permit2.rs b/src/encoding/evm/approvals/permit2.rs index 7c23732..78c6bfe 100644 --- a/src/encoding/evm/approvals/permit2.rs +++ b/src/encoding/evm/approvals/permit2.rs @@ -39,16 +39,19 @@ type Allowance = (U160, U48, U48); // (amount, expiration, nonce) const PERMIT_EXPIRATION: u64 = 30 * 24 * 60 * 60; /// Expiration period for signatures, set to 30 minutes (in seconds). const PERMIT_SIG_EXPIRATION: u64 = 30 * 60; +const MAX_UINT48: U48 = U48::MAX; +const MAX_UINT160: U160 = U160::MAX; +const MAX_UINT256: U256 = U256::MAX; sol! { - #[derive(PartialEq, Debug)] + #[derive(Debug)] struct PermitSingle { PermitDetails details; address spender; uint256 sigDeadline; } - #[derive(PartialEq, Debug)] + #[derive(Debug)] struct PermitDetails { address token; uint160 amount; @@ -62,7 +65,7 @@ impl Permit2 { pub fn new(signer: PrivateKeySigner, chain_id: ChainId) -> Result { let runtime = Runtime::new() .map_err(|_| EncodingError::FatalError("Failed to create runtime".to_string()))?; - let client = runtime.block_on(get_client()); + let client = runtime.block_on(get_client())?; Ok(Self { address: Address::from_str("0x000000000022D473030F116dDEE9F6B43aC78BA3") .map_err(|_| EncodingError::FatalError("Permit2 address not valid".to_string()))?, @@ -74,7 +77,7 @@ impl Permit2 { } /// Fetches allowance data for a specific owner, spender, and token. - fn get_allowance_data( + fn get_existing_allowance( &self, owner: &Bytes, spender: &Bytes, @@ -120,26 +123,32 @@ impl UserApprovalsManager for Permit2 { for approval in approvals { let (_, _, nonce) = - self.get_allowance_data(&approval.owner, &approval.spender, &approval.token)?; - let expiration = current_time + PERMIT_EXPIRATION; - let sig_deadline = current_time + PERMIT_SIG_EXPIRATION; + self.get_existing_allowance(&approval.owner, &approval.spender, &approval.token)?; + let expiration = U48::from(current_time + PERMIT_EXPIRATION); + let sig_deadline = U256::from(current_time + PERMIT_SIG_EXPIRATION); + let amount = U160::from(biguint_to_u256(&approval.amount)); + + // validations + assert!(MAX_UINT256 > sig_deadline, "signature deadline out of range"); + assert!(MAX_UINT48 > nonce, "nonce out of range"); + assert!(MAX_UINT160 > amount, "amount out of range"); + assert!(MAX_UINT48 > expiration, "expiration out of range"); let details = PermitDetails { token: bytes_to_address(&approval.token)?, - amount: U160::from(biguint_to_u256(&approval.amount)), - expiration: U48::from(expiration), + amount, + expiration, nonce, }; let permit_single = PermitSingle { details, spender: bytes_to_address(&approval.spender)?, - sigDeadline: U256::from(sig_deadline), + sigDeadline: sig_deadline, }; - let mut encoded = permit_single.abi_encode(); let domain = eip712_domain! { - name: "Permit", + name: "Permit2", chain_id: self.chain_id, verifying_contract: self.address, }; @@ -153,8 +162,9 @@ impl UserApprovalsManager for Permit2 { e )) })?; - - encoded.extend(signature.as_bytes()); + let encoded = + (bytes_to_address(&approval.owner)?, permit_single, signature.as_bytes().to_vec()) + .abi_encode(); encoded_approvals.push(encoded); } @@ -170,6 +180,38 @@ mod tests { use num_bigint::BigUint; use super::*; + + // These two implementations are to avoid comparing the expiration and sig_deadline fields + // because they are timestamps + impl PartialEq for PermitSingle { + fn eq(&self, other: &Self) -> bool { + if self.details != other.details { + return false; + } + if self.spender != other.spender { + return false; + } + true + } + } + + impl PartialEq for PermitDetails { + fn eq(&self, other: &Self) -> bool { + if self.token != other.token { + return false; + } + if self.amount != other.amount { + return false; + } + // Compare `nonce` + if self.nonce != other.nonce { + return false; + } + + true + } + } + #[test] fn test_get_allowance_data() { let signer = PrivateKeySigner::random(); @@ -180,7 +222,7 @@ mod tests { let spender = Bytes::from_str("0xba12222222228d8ba445958a75a0704d566bf2c8").unwrap(); let result = manager - .get_allowance_data(&owner, &spender, &token) + .get_existing_allowance(&owner, &spender, &token) .unwrap(); assert_eq!( result, @@ -210,12 +252,8 @@ mod tests { let encoded = &encoded_approvals[0]; - // Calculate the PermitSingle ABI-encoded length - let permit_details_length = 32 + 32 + 32 + 32; // token + amount + expiration + nonce - let permit_single_length = permit_details_length + 32 + 32; // details + spender + sigDeadline - let (permit_single_encoded, signature_encoded) = encoded.split_at(permit_single_length); - - assert_eq!(signature_encoded.len(), 65, "Expected 65 bytes for signature"); + // Remove prefix and owner (first 64 bytes) and signature (last 65 bytes) + let permit_single_encoded = &encoded[64..encoded.len() - 65]; let decoded_permit_single = PermitSingle::abi_decode(permit_single_encoded, false) .expect("Failed to decode PermitSingle"); @@ -237,4 +275,87 @@ mod tests { "Decoded PermitSingle does not match expected values" ); } + + /// This test actually calls the permit method on the Permit2 contract to verify the encoded + /// data works. It requires an Anvil fork, so please run with the following command: anvil + /// --fork-url And set up the following env var as ETH_RPC_URL=127.0.0.1:8545 + /// Use an account from anvil to fill the anvil_account and anvil_private_key variables + #[test] + #[cfg_attr(not(feature = "fork-tests"), ignore)] + fn test_permit() { + let anvil_account = Bytes::from_str("0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266").unwrap(); + let anvil_private_key = + B256::from_str("0xac0974bec39a17e36ba4a6b4d238ff944bacb478cbed5efcae784d7bf4f2ff80") + .unwrap(); + + let signer = + PrivateKeySigner::from_bytes(&anvil_private_key).expect("Failed to create signer"); + let permit2 = Permit2::new(signer, 1).expect("Failed to create Permit2"); + + let token = Bytes::from_str("0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48").unwrap(); + let amount = BigUint::from(1000u64); + + // Approve token allowance for permit2 contract + let approve_function_signature = "approve(address,uint256)"; + let args = (permit2.address, biguint_to_u256(&BigUint::from(1000000u64))); + let data = encode_input(approve_function_signature, args.abi_encode()); + + let tx = TransactionRequest { + to: Some(TxKind::from(bytes_to_address(&token).unwrap())), + input: TransactionInput { input: Some(AlloyBytes::from(data)), data: None }, + ..Default::default() + }; + let receipt = permit2.runtime.block_on(async { + let pending_tx = permit2 + .client + .send_transaction(tx) + .await + .unwrap(); + // Wait for the transaction to be mined + pending_tx.get_receipt().await.unwrap() + }); + assert!(receipt.status(), "Approve transaction failed"); + + let spender = Bytes::from_str("0xba12222222228d8ba445958a75a0704d566bf2c8").unwrap(); + let approvals = vec![Approval { + owner: anvil_account.clone(), + spender: spender.clone(), + token: token.clone(), + amount: amount.clone(), + }]; + + let encoded_approvals = permit2 + .encode_approvals(approvals) + .unwrap(); + + let encoded = &encoded_approvals[0]; + + let function_signature = + "permit(address,((address,uint160,uint48,uint48),address,uint256),bytes)"; + let data = encode_input(function_signature, encoded.to_vec()); + + let tx = TransactionRequest { + to: Some(TxKind::from(permit2.address)), + input: TransactionInput { input: Some(AlloyBytes::from(data)), data: None }, + gas: Some(10_000_000u64), + ..Default::default() + }; + + let result = permit2.runtime.block_on(async { + let pending_tx = permit2 + .client + .send_transaction(tx) + .await + .unwrap(); + pending_tx.get_receipt().await.unwrap() + }); + assert!(result.status(), "Permit transaction failed"); + + // Assert that the allowance was set correctly in the permit2 contract + let (allowance_amount, _, nonce) = permit2 + .get_existing_allowance(&anvil_account, &spender, &token) + .unwrap(); + assert_eq!(allowance_amount, U160::from(biguint_to_u256(&amount))); + assert_eq!(nonce, U48::from(1)); + } }