feat: alloy-dyn-contract #149
Conversation
onbjerg
requested review from
Evalir,
prestwich,
DaniPopes and
gakonst
as code owners
| pub struct CallBuilder<P> { | ||
| // todo: this will not work with `send_transaction` and does not differentiate between EIP-1559 | ||
| // and legacy tx | ||
| request: CallRequest, |
There was a problem hiding this comment.
I think having access to the CallRequest would be ideal. This way we can get around not having send_transaction on the provider by transferring the fields manually to a signable/rlp-able tx type from alloy consensus e.g TxLegacy
| /// A smart contract interface. | ||
| #[derive(Debug, Clone)] |
There was a problem hiding this comment.
Could we add a bit more docs on why it's necessary/more ergonomic to have this type vs extending JsonAbi?
| } | ||
| } | ||
|
|
||
| // TODO: events/errors |
There was a problem hiding this comment.
yea we don't have the facilities rn for events so it's more of a follow up to do
There was a problem hiding this comment.
sweet np, let's open an issue for this and roll
| @@ -0,0 +1,123 @@ | |||
| use std::collections::HashMap; | |||
There was a problem hiding this comment.
This file LGTM. I trust its functionality and would defer writing tests, open as issue and we can pass it to contributor as this is code that just wraps around JsonABI.
| /// If there are multiple functions with the same name due to overloading, consider using | ||
| /// the [`ContractInstance::function_from_selector`] method instead, since this will use the | ||
| /// first match. | ||
| pub fn function(&self, name: &str, args: &[DynSolValue]) -> Result<CallBuilder<P>> { |
There was a problem hiding this comment.
I would love if we had a great UX about this. Right now it'd require the user to manually do DynSolValue::X for each argument. The way we did it with the generics Tokenize in Ethers-rs was nice, but understand if it's a PITA to work with.
cc @DaniPopes
There was a problem hiding this comment.
I tried to convey that at last sync but there did not seem to be a lot of agreement
| pub async fn call(&self) -> Result<Vec<DynSolValue>> { | ||
| let bytes = self | ||
| .provider | ||
| .call(self.request.clone(), self.block, self.state.clone()) |
There was a problem hiding this comment.
@DaniPopes @mattsse is there a world where we can get rid of these clones and use refs under the hood
| block: Option<BlockId>, | ||
| state: Option<StateOverride>, | ||
| provider: P, | ||
| // todo: only used to decode - should it be some type D to dedupe with `sol!` contracts? |
There was a problem hiding this comment.
Will need a follow-up for this so we can also use it in sol!
|
will merge as is and we can make improvements on top, we're prob bound to have missed something from the old api that someone has a use case for |
Motivation
We want something like
BaseContract/ContractInstancein ethers-rs for dynamic ABI in alloy.Solution
Port over what we can of those existing APIs to alloy (i.e. really only
call) and simplify theCallBuildera bitIn ethers there are 2 call builders, a main one,
FunctionCall, and a secondary one you can get fromFunctionCall(calledRawCall) which you can use to set state overrides. I inlined the state override API intoCallBuilderinstead, and opted to only add the 3rd parameter foreth_callif state overrides are set, as this is not supported by all nodes.I'm not sure if there is any risk in sending a third "bogus" parameter in
eth_call, but if so, maybe the API in this PR could be updated to split out theeth_call+ state override to a different function in the provider.This also does not include sending transactions (that part is a
todo!()) because that flow is not really done yet in the provider or the signer.A separate pr for "static" contracts should be opened
This PR does also not differentiate between legacy/EIP-1559 tx's as we don't have a "typed tx" construct in Alloy yet
PR Checklist