Skip to content

feat: alloy-contract#182

Merged
DaniPopes merged 11 commits intomainfrom
dani/sol-contract
Feb 7, 2024
Merged

feat: alloy-contract#182
DaniPopes merged 11 commits intomainfrom
dani/sol-contract

Conversation

@DaniPopes
Copy link
Member

@DaniPopes DaniPopes commented Feb 5, 2024

Upgrade alloy-dyn-contract to alloy-contract by making CallBuilder generic over the output decoder, which can now be sol!-generated (see alloy-rs/core#510) as well "noop" decoder which makes call behave the same as call_raw.

Bumps MSRV to 1.75, but this is not strictly necessary for the core functionality.

Comment on lines +189 to +191
// TODO: this will not work with `send_transaction` and does not differentiate between EIP-1559
// and legacy tx
request: CallRequest,
Copy link
Member

Choose a reason for hiding this comment

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

right for signing we need another step where the request is converted into a TypedTransaction first

Copy link
Contributor

Choose a reason for hiding this comment

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

Related TypedTransaction work: #183

Comment on lines +353 to +356
/// Signs and broadcasts this call as a transaction.
pub async fn send(&self) -> Result<()> {
todo!()
}
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps we can move this behind a Provider trait bound, requiring that the provider supports signing

Copy link
Member

Choose a reason for hiding this comment

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

we want it to work with eth_sendTransaction too, so i think just need to modify the comment

Comment on lines +14 to +22
/// [`CallBuilder`] using a [`SolCall`] type as the call decoder.
// NOTE: please avoid changing this type due to its use in the `sol!` macro.
pub type SolCallBuilder<P, C> = CallBuilder<P, PhantomData<C>>;

/// [`CallBuilder`] using a [`Function`] as the call decoder.
pub type DynCallBuilder<P> = CallBuilder<P, Function>;

/// [`CallBuilder`] that does not have a call decoder.
pub type RawCallBuilder<P> = CallBuilder<P, ()>;
Copy link
Member

Choose a reason for hiding this comment

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

amazing -- exactly how i had it in mind

Copy link
Member

@onbjerg onbjerg left a comment

Choose a reason for hiding this comment

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

lgtm (minus doc nits from other reviewers) and also noting that the SolCallBuilder type will end up changing pretty soon

@DaniPopes
Copy link
Member Author

DaniPopes commented Feb 6, 2024

CallBuilder internals changing is fine, as long as the APIs marked with my NOTE comment stay the same, so as to not break sol!-generated code. Otherwise it would need to be updated in alloy-rs/core too.

@onbjerg
Copy link
Member

onbjerg commented Feb 6, 2024

SolCallBuilder will end up taking an additional generic for N: Network, so this would break the API.

@DaniPopes DaniPopes requested review from mattsse and onbjerg February 7, 2024 01:32
Copy link
Member

@onbjerg onbjerg left a comment

Choose a reason for hiding this comment

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

pog personally, this looks good to me. excited to see this macro in the wild

need to fix deny check, otherwise can merge imo

@DaniPopes DaniPopes merged commit 1737429 into main Feb 7, 2024
@DaniPopes DaniPopes deleted the dani/sol-contract branch February 7, 2024 18:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants