[ingress] add transaction validation #16
Conversation
wlawt
force-pushed
the
transaction-validation
branch
from
dff3678 to
4f8afcb
Compare
wlawt
force-pushed
the
transaction-validation
branch
from
8c87a6c to
bd9e053
Compare
crates/ingress-rpc/src/service.rs
Outdated
| } | ||
|
|
||
| // error if execution cost costs more than balance | ||
| if envelope.value() > account.balance { |
There was a problem hiding this comment.
i'm not sure if value() is correct here.
i also don't know if it's possible to get the cost() from an OpPooledTransaction or Op envelope since these functions are generated from a macro, it's hard to trace exactly what's available
There was a problem hiding this comment.
I suspect this is some combo of gasPrice * gasLimit + value. We should try and import and if not we can reimplement
There was a problem hiding this comment.
will re-implement cost().
i tracked it down: for an EIP-1559 tx, the value() is amount being transferred https://github.com/alloy-rs/alloy/blob/d826a501ee96925c1e67ca87bd4fe819b807fbaf/crates/consensus/src/transaction/eip1559.rs#L64
wlawt
changed the title
danyalprout
left a comment
danyalprout
left a comment
There was a problem hiding this comment.
lgtm, few small thoughts. would it be possible to add some tests to cover these cases too 🙏
crates/ingress-rpc/src/service.rs
Outdated
|
|
||
| // skip eip4844 transactions | ||
| if envelope.is_eip4844() { | ||
| return Err(anyhow::anyhow!("EIP-4844 transactions are not supported")); |
There was a problem hiding this comment.
Are there typed errors for these?
There was a problem hiding this comment.
there are, but it's spread across different enums like EthApiError, RpcInvalidTransactionError etc. but will consolidate some using the convenient RpcInvalidTransactionError::other available for error types not mentioned in the enum
crates/ingress-rpc/src/service.rs
Outdated
| } | ||
|
|
||
| // error if execution cost costs more than balance | ||
| if envelope.value() > account.balance { |
There was a problem hiding this comment.
I suspect this is some combo of gasPrice * gasLimit + value. We should try and import and if not we can reimplement
* wip * no more trait errors * impl into provider as trait * nits
crates/ingress-rpc/src/service.rs
Outdated
| .map_err(|e| ErrorObject::owned(11, e.to_string(), Some(2)))?; | ||
|
|
||
| let account = self | ||
| .provider | ||
| .fetch_account_info(transaction.signer()) | ||
| .await | ||
| .map_err(|e| ErrorObject::owned(11, e.to_string(), Some(2)))?; | ||
| validate_tx(account, &transaction, &data, &mut l1_block_info) | ||
| .await | ||
| .map_err(|e| ErrorObject::owned(11, e.to_string(), Some(2)))?; |
There was a problem hiding this comment.
Is there any typed errors/error codes that we could pull out here? This should match the node implementation as much as possible vs. just being 11
Overview
A successful validation of transaction is:
TODO
Tests