-
Notifications
You must be signed in to change notification settings - Fork 17
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
Improve API consistency #85
Merged
Merged
Changes from all commits
Commits
Show all changes
30 commits
Select commit
Hold shift + click to select a range
b824eb2
Add withdraw request builder
IAvecilla 710c4b7
New module for specific transaction requests
IAvecilla ec28030
Improve return type on send functions
IAvecilla 8199608
Delete duplicated code to get providers
IAvecilla 2ef7d7a
Add function to provider to send eip712 transactions
IAvecilla 981cfe3
Add from trait for eip712 transactions with every request type
IAvecilla c669388
Revert returning transaction hash from send functions
IAvecilla bd40b5b
Add transfer request type
IAvecilla 01789f8
Add defaults for withdraw request
IAvecilla cb2f4f1
Update wallet to use every transaction type
IAvecilla 9e9d92f
Add deploy request implementation
IAvecilla 3cbd7b0
Use deploy request in wallet deploy function
IAvecilla 5ab4ec7
Add call request implementation
IAvecilla b29a21a
Fix unused imports
IAvecilla 01d399f
Fix deploy test
IAvecilla 85818be
Fix clippy lints
IAvecilla 6d3a8b3
Add request conversion error
IAvecilla aa399f4
Fix unused import
IAvecilla 760cd5d
Update README and payment example
IAvecilla f33c543
Return contract address in deploy function
IAvecilla cda5391
Remove unnecesary sleeps in tests
IAvecilla eef156c
Merge branch 'main' into improve_api
IAvecilla be1b6b8
Merge branch 'main' into improve_api
IAvecilla d510941
Update request builder methods
IAvecilla 75ec65d
Change return type to pending transaction
IAvecilla 97b5125
Return transaction ID in every wallet operation
IAvecilla 995813a
Update README and payment example
IAvecilla 2e30826
Empty commit
IAvecilla 238066b
Merge branch 'main' into improve_api
IAvecilla 79e727a
Merge branch 'main' into improve_api
IAvecilla File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,15 +1,21 @@ | ||
use super::{rlp_append_option, Eip712Meta}; | ||
use std::{fs::File, io::BufReader, path::PathBuf, str::FromStr}; | ||
|
||
use super::{hash_bytecode, rlp_append_option, Eip712Meta}; | ||
use crate::{ | ||
zks_utils::{EIP712_TX_TYPE, ERA_CHAIN_ID, MAX_PRIORITY_FEE_PER_GAS}, | ||
zks_wallet::Overrides, | ||
zks_utils::{ | ||
self, CONTRACT_DEPLOYER_ADDR, EIP712_TX_TYPE, ERA_CHAIN_ID, MAX_PRIORITY_FEE_PER_GAS, | ||
}, | ||
zks_wallet::{DeployRequest, Overrides, TransferRequest, WithdrawRequest, ZKRequestError}, | ||
}; | ||
use ethers::{ | ||
abi::{Abi, HumanReadableParser, ParseError}, | ||
types::{ | ||
transaction::{eip2930::AccessList, eip712::Eip712Error}, | ||
Address, Bytes, Signature, U256, | ||
}, | ||
utils::rlp::{Encodable, RlpStream}, | ||
}; | ||
use ethers_contract::encode_function_data; | ||
use serde::{Deserialize, Serialize}; | ||
|
||
// TODO: Not all the fields are optional. This was copied from the JS implementation. | ||
|
@@ -233,3 +239,98 @@ impl Default for Eip712TransactionRequest { | |
} | ||
} | ||
} | ||
|
||
impl TryFrom<WithdrawRequest> for Eip712TransactionRequest { | ||
type Error = ZKRequestError; | ||
|
||
fn try_from(request: WithdrawRequest) -> Result<Self, Self::Error> { | ||
let contract_address = | ||
Address::from_str(zks_utils::CONTRACTS_L2_ETH_TOKEN_ADDR).map_err(|e| { | ||
ZKRequestError::CustomError(format!("Error getting L2 ETH token address {e:?}")) | ||
})?; | ||
let function_signature = "function withdraw(address _l1Receiver) external payable override"; | ||
let function = HumanReadableParser::parse_function(function_signature) | ||
.map_err(ParseError::LexerError)?; | ||
let function_args = function.decode_input(&zks_utils::encode_args( | ||
&function, | ||
&[format!("{:?}", request.to)], | ||
)?)?; | ||
let data: Bytes = function.encode_input(&function_args)?.into(); | ||
|
||
Ok(Eip712TransactionRequest::new() | ||
.r#type(EIP712_TX_TYPE) | ||
.to(contract_address) | ||
.value(request.amount) | ||
.from(request.from) | ||
.data(data)) | ||
} | ||
} | ||
|
||
impl From<TransferRequest> for Eip712TransactionRequest { | ||
fn from(request: TransferRequest) -> Self { | ||
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 my comment for |
||
Eip712TransactionRequest::new() | ||
.r#type(EIP712_TX_TYPE) | ||
.to(request.to) | ||
.value(request.amount) | ||
.from(request.from) | ||
} | ||
} | ||
|
||
impl TryFrom<DeployRequest> for Eip712TransactionRequest { | ||
type Error = ZKRequestError; | ||
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 my comment for |
||
|
||
fn try_from(request: DeployRequest) -> Result<Self, Self::Error> { | ||
let mut contract_deployer_path = PathBuf::from(env!("CARGO_MANIFEST_DIR")); | ||
contract_deployer_path.push("src/abi/ContractDeployer.json"); | ||
|
||
let custom_data = Eip712Meta::new().factory_deps({ | ||
let mut factory_deps = Vec::new(); | ||
if let Some(factory_dependencies) = request.factory_deps { | ||
factory_deps.extend(factory_dependencies); | ||
} | ||
factory_deps.push(request.contract_bytecode.clone()); | ||
factory_deps | ||
}); | ||
|
||
let contract_deployer = Abi::load(BufReader::new( | ||
File::open(contract_deployer_path).map_err(|e| { | ||
ZKRequestError::CustomError(format!( | ||
"Error opening contract deployer abi file {e:?}" | ||
)) | ||
})?, | ||
))?; | ||
let create = contract_deployer.function("create")?; | ||
|
||
// TODO: User could provide this instead of defaulting. | ||
let salt = [0_u8; 32]; | ||
let bytecode_hash = hash_bytecode(&request.contract_bytecode).map_err(|e| { | ||
ZKRequestError::CustomError(format!("Error hashing contract bytecode {e:?}")) | ||
})?; | ||
let call_data: Bytes = match ( | ||
request.contract_abi.constructor(), | ||
request.constructor_parameters.is_empty(), | ||
) { | ||
(None, false) => { | ||
return Err(ZKRequestError::CustomError( | ||
"Constructor not present".to_owned(), | ||
)) | ||
} | ||
(None, true) | (Some(_), true) => Bytes::default(), | ||
(Some(constructor), false) => { | ||
zks_utils::encode_constructor_args(constructor, &request.constructor_parameters)? | ||
.into() | ||
} | ||
}; | ||
|
||
let data = encode_function_data(create, (salt, bytecode_hash, call_data))?; | ||
|
||
let contract_deployer_address = Address::from_str(CONTRACT_DEPLOYER_ADDR).map_err(|e| { | ||
ZKRequestError::CustomError(format!("Error getting contract deployer address {e:?}")) | ||
})?; | ||
Ok(Eip712TransactionRequest::new() | ||
.r#type(EIP712_TX_TYPE) | ||
.to(contract_deployer_address) | ||
.custom_data(custom_data) | ||
.data(data)) | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Since a
WithdrawRequest
is a subset ofEip712TransactionRequest
s, shouldn't we implement aTryInto
in that module? (following the principle thatEip712TransactionRequest
shouldn't know about its children structures.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.
The
TryFrom
function already implements the functionality ofTryInto
, but the reverse isn't necessarily true. By implementingTryFrom
orFrom
, you automatically gain the correspondingTryInto
orInto
trait respectively. This behavior is outlined in the Rust documentation on this trait.