Skip to content

feat: example code for Fillers #43

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

feat: example code for Fillers #43

wants to merge 5 commits into from

Conversation

anna-carroll
Copy link
Contributor

Example code for Fillers of Signet Orders.

@anna-carroll anna-carroll self-assigned this Apr 30, 2025
@anna-carroll anna-carroll requested a review from a team as a code owner April 30, 2025 14:29
Base automatically changed from anna/to-tx to main May 1, 2025 10:04
use std::collections::HashMap;

/// Multiplier for converting gwei to wei.
const GWEI_TO_WEI: u64 = 1_000_000_000;
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Should use the alloy constant for this value instead of declaring it ourselves.

impl From<Vec<SignedOrder>> for AggregateOrders {
fn from(orders: Vec<SignedOrder>) -> Self {
let mut agg = AggregateOrders::new();
for order in orders {
Copy link
Member

Choose a reason for hiding this comment

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

whole function can be replaced by

.iter().collect()

fn from(orders: Vec<RollupOrders::Order>) -> Self {
let mut agg = AggregateOrders::new();
for order in orders {
agg.ingest(&order);
Copy link
Member

Choose a reason for hiding this comment

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

whole function can be replaced by

.iter().collect()

/// Sign them and encode them for inclusion in a Bundle.
pub async fn sign_and_encode_txns(
&self,
tx_requests: Vec<TransactionRequest>,
Copy link
Member

@prestwich prestwich May 1, 2025

Choose a reason for hiding this comment

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

almost all of your methods should take &[T] instead of Vec<T>

async fn rollup_txn_requests(
&self,
signed_fills: &HashMap<u64, SignedFill>,
orders: &Vec<SignedOrder>,
Copy link
Member

Choose a reason for hiding this comment

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

&[SignedOrder]

/// rather than signing a single, aggregate a Fill for each chain, as is done here.
pub async fn sign_fills(
&self,
orders: Vec<SignedOrder>,
Copy link
Member

Choose a reason for hiding this comment

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

&[SignedOrder]

/// If a single Order is passed to this fn,
/// Filling Orders individually ensures that even if some Orders are not fillable, others may still mine;
/// however, it is less gas efficient.
pub async fn fill(&self, orders: Vec<SignedOrder>) -> Result<(), Error> {
Copy link
Member

Choose a reason for hiding this comment

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

&[SignedOrder]

///
/// Filling Orders individually ensures that even if some Orders are not fillable, others may still mine;
/// however, it is less gas efficient.
pub async fn fill_individual(&self, orders:Vec<SignedOrder>) -> Result<(), Error> {
Copy link
Member

Choose a reason for hiding this comment

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

&[SignedOrder]

let txs = self.sign_and_encode_txns(tx_requests).await?;

// get the aggregated host fill for the Bundle, if any
let host_fills = signed_fills.get(&self.host_chain_id).cloned();
Copy link
Member

Choose a reason for hiding this comment

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

you don't need this anymore, so you can remove here to avoid cloning

}

// sign a SignedFill for the orders
let signed_fills = self.sign_fills(orders.clone()).await?;
Copy link
Member

Choose a reason for hiding this comment

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

change to borrows so no need to clone

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.

4 participants