Skip to content
This repository has been archived by the owner on Nov 5, 2023. It is now read-only.

feat: fix syncing errors #83

Merged
merged 9 commits into from
Aug 20, 2023
Merged

feat: fix syncing errors #83

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
a4a6574
Start more fixes
clabby Aug 20, 2023
f8f2c4d
test
clabby Aug 20, 2023
f3463d1
wat do
clabby Aug 20, 2023
dd9712b
Not input, encoded enveloped :sweat_smile:
clabby Aug 20, 2023
3b225db
Remove debug logs
clabby Aug 20, 2023
0480cb6
Clean up optimism payload builder
clabby Aug 20, 2023
0f6201d
Add failed deposit receipts in optimism payload builder
clabby Aug 20, 2023
c62e4cd
dbg
clabby Aug 20, 2023
088e28f
lint
clabby Aug 20, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions crates/payload/basic/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,6 @@ where

#[cfg(feature = "optimism")]
if config.attributes.no_tx_pool {
dbg!("No tx pool; Building");
let args = BuildArguments {
client: self.client.clone(),
pool: self.pool.clone(),
Expand All @@ -186,7 +185,6 @@ where
};
match self.builder.try_build(args)? {
BuildOutcome::Better { payload, cached_reads } => {
dbg!("Got better payload", &payload);
return Ok(BasicPayloadJob {
config,
client: self.client.clone(),
Expand Down Expand Up @@ -491,8 +489,6 @@ where
empty_payload = Some(rx);
}

dbg!(&best_payload, &maybe_better, &empty_payload);

let fut = ResolveBestPayload { best_payload, maybe_better, empty_payload };

(fut, KeepPayloadJobAlive::No)
Expand Down
159 changes: 103 additions & 56 deletions crates/payload/basic/src/optimism.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,12 @@ where
let is_regolith =
chain_spec.is_fork_active_at_timestamp(Hardfork::Regolith, attributes.timestamp);

// TODO(clabby): Add new error type
// Parse the L1 block info from the first transaction in the payload attributes. This
// transaction should always be the L1 info tx. We skip the first 4 bytes of the calldata
// because the first 4 bytes are the function selector.
let l1_block_info = (!attributes.transactions.is_empty()).then_some(
L1BlockInfo::try_from(&attributes.transactions[0].input()[..])
.map_err(|_| PayloadBuilderError::TransactionEcRecoverFailed)?,
L1BlockInfo::try_from(&attributes.transactions[0].input()[4..])
.map_err(|_| PayloadBuilderError::L1BlockInfoParseFailed)?,
);

// Transactions sent via the payload attributes are force included at the top of the block, in
Expand All @@ -77,44 +79,67 @@ where
.try_into_ecrecovered()
.map_err(|_| PayloadBuilderError::TransactionEcRecoverFailed)?;

// Compute the L1 cost of the transaction. This is the amount of ETH that it will cost to
// post the entire encoded typed transaction to L1.
let mut encoded = BytesMut::default();
sequencer_tx.encode_enveloped(&mut encoded);
let l1_cost = l1_block_info.as_ref().map(|l1_block_info| {
l1_block_info.calculate_tx_l1_cost(
Arc::clone(&chain_spec),
attributes.timestamp,
sequencer_tx.input(),
&encoded.freeze().into(),
sequencer_tx.is_deposit(),
)
});

let mut cfg = initialized_cfg.clone();

let sender = db.load_account(sequencer_tx.signer())?.clone();
let mut sender_new = sender.clone();

// If the transaction is a deposit, we need to disable the base fee, balance check, and
// gas refund. We also need to disable the block gas limit if the Regolith hardfork is not
// active. In addition, we need to increase the sender's balance by the mint value of the
// deposit transaction if it is `Some(n)`.
//
// Otherwise, we need to decrement the sender's balance by the L1 cost of the transaction
// prior to execution.
if sequencer_tx.is_deposit() {
cfg.disable_base_fee = true;
cfg.disable_balance_check = true;
cfg.disable_gas_refund = true;

// If the Regolith hardfork is active, we do not need to disable the block gas limit.
// Otherwise, we allow for the block gas limit to be exceeded by deposit transactions.
if !is_regolith && sequencer_tx.is_deposit() {
if !is_regolith {
cfg.disable_block_gas_limit = true;
};
}

// Temporarily increase the sender's balance in the database if the deposit transaction
// mints eth.
// Increase the sender's balance in the database if the deposit transaction mints eth.
if let Some(m) = sequencer_tx.mint() {
let sender = db.load_account(sequencer_tx.signer())?.clone();
let mut sender_new = sender.clone();
sender_new.info.balance += U256::from(m);
let m = U256::from(m);
sender_new.info.balance += m;

executor::increment_account_balance(
&mut db,
&mut post_state,
parent_block.number + 1,
sequencer_tx.signer(),
U256::from(m),
m,
)?;
db.insert_account_info(sequencer_tx.signer(), sender_new.info);
}
} else if let Some(l1_cost) = l1_cost {
// Decrement the sender's balance by the L1 cost of the transaction prior to execution.
sender_new.info.balance -= l1_cost;
executor::decrement_account_balance(
&mut db,
&mut post_state,
parent_block.number + 1,
sequencer_tx.signer(),
l1_cost,
)?;
db.insert_account_info(sequencer_tx.signer(), sender_new.info);
}

// Configure the environment for the block.
Expand All @@ -130,14 +155,34 @@ where
let ResultAndState { result, state } = match evm.transact() {
Ok(res) => res,
Err(err) => {
// TODO(clabby): This could be an issue - deposit transactions should always be
// included with a receipt regardless of if there was an error or not. The
// sequencer performs some basic validation on the transactions it sends prior
// to sending a fork choice update, so this shouldn't be an issue, but we may
// want to revisit this.
if sequencer_tx.is_deposit() {
post_state.add_receipt(
block_number,
Receipt {
tx_type: sequencer_tx.tx_type(),
success: false,
cumulative_gas_used,
logs: vec![],
deposit_nonce: if is_regolith && sequencer_tx.is_deposit() {
// Recovering the signer from the deposit transaction is only
// fetching the `from` address.
// Deposit transactions have no signature.
let from = sequencer_tx.signer();
let account = db.load_account(from)?;
// The deposit nonce is the account's nonce - 1. The account's nonce
// was incremented during the execution of the deposit transaction
// above.
Some(account.info.nonce.saturating_sub(1))
} else {
None
},
},
);
continue
}

match err {
EVMError::Transaction(err) => {
dbg!("Transaction error", err);
if matches!(err, InvalidTransaction::NonceTooLow { .. }) {
// if the nonce is too low, we can skip this transaction
trace!(?err, ?sequencer_tx, "skipping nonce too low transaction");
Expand All @@ -153,26 +198,33 @@ where
continue
}
err => {
dbg!("EVM Error", &err);
// this is an error that we should treat as fatal for this attempt
return Err(PayloadBuilderError::EvmExecutionError(err))
}
}
}
};
dbg!("EXECUTED ", sequencer_tx.hash());

// commit changes
commit_state_changes(&mut db, &mut post_state, block_number, state, true);

// Skip coinbase payments in Regolith for deposit transactions
if chain_spec.optimism {
// If either Regolith is active or the transaction is not a deposit, we report the
// actual gas used in execution to the cumulative gas.
//
// Otherwise, if we are pre-Regolith and the transaction is a non-system-tx deposit,
// we report the gas limit of the transaction to the cumulative gas. Pre-Regolith,
// system transactions are not included in the cumulative gas and their execution
// gas is ignored. Post regolith, system transactions are deprecated and no longer
// exist.
if is_regolith || !sequencer_tx.is_deposit() {
cumulative_gas_used += result.gas_used()
} else if sequencer_tx.is_deposit() && !sequencer_tx.is_system_transaction() {
cumulative_gas_used += sequencer_tx.gas_limit();
}
if !(sequencer_tx.is_deposit() && is_regolith) {

// If the transaction is not a deposit, we route the l1 cost and base fee to the
// appropriate optimism vaults.
if !sequencer_tx.is_deposit() {
// Route the l1 cost and base fee to the appropriate optimism vaults
if let Some(l1_cost) = l1_cost {
executor::increment_account_balance(
Expand All @@ -183,43 +235,38 @@ where
l1_cost,
)?
}
if !sequencer_tx.is_deposit() {
executor::increment_account_balance(
&mut db,
&mut post_state,
parent_block.number + 1,
executor::optimism::base_fee_recipient(),
U256::from(base_fee.saturating_mul(result.gas_used())),
)?;
}
executor::increment_account_balance(
&mut db,
&mut post_state,
parent_block.number + 1,
executor::optimism::base_fee_recipient(),
U256::from(base_fee.saturating_mul(result.gas_used())),
)?;
}
}

let r = Receipt {
tx_type: sequencer_tx.tx_type(),
success: result.is_success(),
cumulative_gas_used,
logs: result.logs().into_iter().map(into_reth_log).collect(),
deposit_nonce: if is_regolith && sequencer_tx.is_deposit() {
// Recovering the signer from the deposit transaction is only fetching
// the `from` address. Deposit transactions have no signature.
let from = sequencer_tx.signer();
let account = db.load_account(from)?;
// The deposit nonce is the account's nonce - 1. The account's nonce
// was incremented during the execution of the deposit transaction
// above.
Some(account.info.nonce.saturating_sub(1))
} else {
None
},
};

dbg!(&r, result);

// Push transaction changeset and calculate header bloom filter for receipt.
post_state.add_receipt(block_number, r);

dbg!("COMMITTED STATE CHANGES");
post_state.add_receipt(
block_number,
Receipt {
tx_type: sequencer_tx.tx_type(),
success: result.is_success(),
cumulative_gas_used,
logs: result.logs().into_iter().map(into_reth_log).collect(),
deposit_nonce: if is_regolith && sequencer_tx.is_deposit() {
// Recovering the signer from the deposit transaction is only fetching
// the `from` address. Deposit transactions have no signature.
let from = sequencer_tx.signer();
let account = db.load_account(from)?;
// The deposit nonce is the account's nonce - 1. The account's nonce
// was incremented during the execution of the deposit transaction
// above.
Some(account.info.nonce.saturating_sub(1))
} else {
None
},
},
);

// append transaction to the list of executed transactions
executed_txs.push(sequencer_tx.into_signed());
Expand Down
5 changes: 5 additions & 0 deletions crates/payload/builder/src/error.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,11 @@ pub enum PayloadBuilderError {
#[cfg(feature = "optimism")]
#[error("failed to convert deposit transaction to TransactionSignedEcRecovered")]
TransactionEcRecoverFailed,
/// Thrown when the L1 block info could not be parsed from the calldata of the
/// first transaction supplied in the payload attributes.
#[cfg(feature = "optimism")]
#[error("failed to parse L1 block info from L1 info tx calldata")]
L1BlockInfoParseFailed,
}

impl From<oneshot::error::RecvError> for PayloadBuilderError {
Expand Down
Loading
Loading