Skip to content

multicall fallback + improvements #11

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.

Sign up for GitHub

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

Merged
merged 5 commits into from
Jul 8, 2025
Merged

multicall fallback + improvements #11

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
8996aa2
skip unncessarily verifying erc1271 signature + remove stray dbg
d4mr Jul 8, 2025
e3ea9cb
clippy
d4mr Jul 8, 2025
f10c10e
add schema titles
d4mr Jul 8, 2025
9583209
fix error reporting for erc1271 verification
d4mr Jul 8, 2025
0dde05e
Add `serde_with` dependency and implement multicall configuration in …
d4mr Jul 8, 2025
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
1 change: 1 addition & 0 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

61 changes: 29 additions & 32 deletions aa-core/src/signer.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,12 @@ use alloy::{
dyn_abi::TypedData,
primitives::{Address, B256, hex, keccak256},
sol,
sol_types::{SolCall, SolValue, decode_revert_reason, eip712_domain},
sol_types::{SolCall, SolValue, eip712_domain},
};
use engine_core::{
chain::Chain,
credentials::SigningCredential,
error::EngineError,
error::{ContractErrorToEngineError, EngineError},
signer::{AccountSigner, EoaSigner, EoaSigningOptions, Erc4337SigningOptions},
};
use serde::Serialize;
Expand Down Expand Up @@ -248,7 +248,7 @@ impl<C: Chain + Clone> SmartAccountSigner<C> {
}

/// Verify ERC-1271 signature
async fn verify_erc1271(&self, hash: B256, signature: &str) -> Result<bool, EngineError> {
pub async fn verify_erc1271(&self, hash: B256, signature: &str) -> Result<bool, EngineError> {
let signature_bytes = hex::decode(signature.strip_prefix("0x").unwrap_or(signature))
.map_err(|_| EngineError::ValidationError {
message: "Invalid signature hex".to_string(),
Expand All @@ -263,14 +263,11 @@ impl<C: Chain + Clone> SmartAccountSigner<C> {
.await
{
Ok(response) => {
dbg!(response);
let expected_magic = ERC1271Contract::isValidSignatureCall::SELECTOR;
Ok(response.as_slice() == expected_magic)
}
Err(e) => {
let data = e.as_revert_data().unwrap();
dbg!(decode_revert_reason(data.as_ref()));
Ok(false)
Err(e.to_engine_error(self.chain.chain_id(), Some(self.smart_account.address)))
}
}
}
Expand Down Expand Up @@ -333,16 +330,16 @@ impl<C: Chain + Clone> SmartAccountSigner<C> {

if is_deployed {
// Verify ERC-1271 signature for deployed accounts
let message_hash = self.hash_message(message, format);
let is_valid = self.verify_erc1271(message_hash, &signature).await?;

if is_valid {
Ok(signature)
} else {
Err(EngineError::ValidationError {
message: "ERC-1271 signature validation failed".to_string(),
})
}
// let message_hash = self.hash_message(message, format);
// let is_valid = self.verify_erc1271(message_hash, &signature).await?;

// if is_valid {
Ok(signature)
// } else {
// Err(EngineError::ValidationError {
// message: "ERC-1271 signature validation failed".to_string(),
// })
// }
} else {
// Create ERC-6492 signature for undeployed accounts
self.create_erc6492_signature(&signature).await
Expand All @@ -359,21 +356,21 @@ impl<C: Chain + Clone> SmartAccountSigner<C> {

if is_deployed {
// Verify ERC-1271 signature for deployed accounts
let typed_data_hash =
typed_data
.eip712_signing_hash()
.map_err(|_e| EngineError::ValidationError {
message: "Failed to compute typed data hash".to_string(),
})?;
let is_valid = self.verify_erc1271(typed_data_hash, &signature).await?;

if is_valid {
Ok(signature)
} else {
Err(EngineError::ValidationError {
message: "ERC-1271 signature validation failed".to_string(),
})
}
// let typed_data_hash =
// typed_data
// .eip712_signing_hash()
// .map_err(|_e| EngineError::ValidationError {
// message: "Failed to compute typed data hash".to_string(),
// })?;
// let is_valid = self.verify_erc1271(typed_data_hash, &signature).await?;

// if is_valid {
Ok(signature)
// } else {
// Err(EngineError::ValidationError {
// message: "ERC-1271 signature validation failed".to_string(),
// })
// }
Comment on lines +359 to +373
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Critical: ERC-1271 signature verification is disabled for typed data.

Similar to the message signing, the ERC-1271 verification for typed data signatures has been commented out, creating the same security risk.

This change should be reverted or properly justified with documentation explaining why verification is being bypassed.

🤖 Prompt for AI Agents 
In aa-core/src/signer.rs between lines 359 and 373, the ERC-1271 signature
verification for typed data is commented out, disabling an important security
check. Restore the commented code that computes the typed data hash, performs
the ERC-1271 verification asynchronously, and conditionally returns the
signature or an error if validation fails. Ensure the verification logic is
active to maintain security unless a clear justification is documented.

} else {
// Create ERC-6492 signature for undeployed accounts
self.create_erc6492_signature(&signature).await
Expand Down
3 changes: 3 additions & 0 deletions core/src/defs.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,19 @@ use schemars::JsonSchema;
use serde::{Deserialize, Serialize};

#[derive(JsonSchema, Serialize, Deserialize, Clone, utoipa::ToSchema)]
#[schema(title = "EVM Address")]
/// ### Address
/// Used to represent an EVM address. This is a string of length 42 with a `0x` prefix. Non-checksummed addresses are also supported, but will be converted to checksummed.
pub struct AddressDef(pub String);

#[derive(JsonSchema, Serialize, Deserialize, Clone, utoipa::ToSchema)]
#[schema(title = "Bytes")]
/// # Bytes
/// Used to represent "bytes". This is a 0x prefixed hex string.
pub struct BytesDef(pub String);

#[derive(JsonSchema, Serialize, Deserialize, Clone, utoipa::ToSchema)]
#[schema(title = "U256")]
/// # U256
/// Used to represent a 256-bit unsigned integer. Engine can parse these from any valid encoding of the Ethereum "quantity" format.
pub struct U256Def(pub String);
1 change: 1 addition & 0 deletions server/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,3 +40,4 @@ utoipa = { version = "5.4.0", features = [
] }
utoipa-axum = "0.2.0"
utoipa-scalar = { version = "0.3.0", features = ["axum"] }
serde_with = "3.14.0"
3 changes: 1 addition & 2 deletions server/src/http/dyn_contract.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,8 +198,7 @@ impl ContractCall {
fn extract_function_name(&self, method: &str) -> Result<String, EngineError> {
let trimmed = method.trim();

if trimmed.starts_with("function ") {
let after_function = &trimmed[9..];
if let Some(after_function) = trimmed.strip_prefix("function ") {
if let Some(paren_pos) = after_function.find('(') {
return Ok(after_function[..paren_pos].trim().to_string());
}
Expand Down
Loading
Loading