Skip to content

Commit

Permalink
addressing PR feedback pt1
Browse files Browse the repository at this point in the history
  • Loading branch information
just-mitch committed Jan 29, 2025
1 parent a367bd3 commit 00da5ff
Show file tree
Hide file tree
Showing 7 changed files with 61 additions and 63 deletions.
29 changes: 6 additions & 23 deletions yarn-project/archiver/src/archiver/data_retrieval.ts
Original file line number Diff line number Diff line change
Expand Up @@ -157,47 +157,30 @@ function extractRollupProposeCalldata(forwarderData: Hex, rollupAddress: Hex): H

// Find all rollup calls
const rollupAddressLower = rollupAddress.toLowerCase();
const rollupCalls = to.reduce((acc: number[], arg, index) => {
if (arg.toLowerCase() === rollupAddressLower) {
acc.push(index);
}
return acc;
}, []);

if (rollupCalls.length === 0) {
throw new Error(`Rollup address not found in forwarder args`);
}

// Find the first propose method among all rollup calls
let rollupData: Hex | undefined;

for (const index of rollupCalls) {
if (index >= data.length) {
for (let i = 0; i < to.length; i++) {
const addr = to[i];
if (addr.toLowerCase() !== rollupAddressLower) {
continue;
}
const callData = data[i];

const callData = data[index];
try {
const { functionName: rollupFunctionName } = decodeFunctionData({
abi: RollupAbi,
data: callData,
});

if (rollupFunctionName === 'propose') {
rollupData = callData;
break;
return callData;
}
} catch (err) {
// Skip invalid function data
continue;
}
}

if (!rollupData) {
throw new Error(`No propose method found in rollup calls`);
}

return rollupData;
throw new Error(`Rollup address not found in forwarder args`);
}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import type { EpochProofQuoteViemArgs } from '@aztec/ethereum';
import { EthAddress } from '@aztec/foundation/eth-address';
import { schemas } from '@aztec/foundation/schemas';
import { BufferReader, serializeToBuffer } from '@aztec/foundation/serialize';
Expand Down Expand Up @@ -98,13 +99,7 @@ export class EpochProofQuotePayload {
.transform(EpochProofQuotePayload.from);
}

toViemArgs(): {
epochToProve: bigint;
validUntilSlot: bigint;
bondAmount: bigint;
prover: `0x${string}`;
basisPointFee: number;
} {
toViemArgs(): EpochProofQuoteViemArgs {
return {
epochToProve: this.epochToProve,
validUntilSlot: this.validUntilSlot,
Expand Down
2 changes: 2 additions & 0 deletions yarn-project/end-to-end/src/e2e_epochs.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,8 @@ describe('e2e_epochs', () => {
monitor = new ChainMonitor(rollup, logger);
monitor.start();

// This is hideous.
// We ought to have a definite reference to the l1TxUtils that we're using in both places, provided by the test context.
proverDelayer = (
((context.proverNode as TestProverNode).publisher as ProverNodePublisher).l1TxUtils as DelayedTxUtils
).delayer!;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,9 @@ const NUM_NODES = 4;
// interfere with each other.
const BOOT_NODE_UDP_PORT = 45000;

const DATA_DIR = './data/gossip';
const DATA_DIR = './data/upgrade_governance_proposer';

jest.setTimeout(1000000);
jest.setTimeout(1000 * 60 * 10);

/**
* This tests emulate the same test as in l1-contracts/test/governance/scenario/UpgradeGovernanceProposerTest.t.sol
Expand Down
34 changes: 24 additions & 10 deletions yarn-project/ethereum/src/contracts/forwarder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,16 +83,30 @@ export class ForwarderContract {
const stats = await l1TxUtils.getTransactionStats(receipt.transactionHash);
return { receipt, gasPrice, stats };
} else {
const errorMsg = await l1TxUtils.tryGetErrorFromRevertedTx(
data,
{
args: [toArgs, dataArgs],
functionName: 'forward',
abi: ForwarderAbi,
address: this.forwarder.address,
},
blobConfig,
);
const args = {
args: [toArgs, dataArgs],
functionName: 'forward',
abi: ForwarderAbi,
address: this.forwarder.address,
};

let errorMsg: string | undefined;

if (blobConfig) {
const maxFeePerBlobGas = blobConfig.maxFeePerBlobGas ?? gasPrice.maxFeePerBlobGas;
if (maxFeePerBlobGas === undefined) {
errorMsg = 'maxFeePerBlobGas is required to get the error message';
} else {
errorMsg = await l1TxUtils.tryGetErrorFromRevertedTx(data, args, {
blobs: blobConfig.blobs,
kzg: blobConfig.kzg,
maxFeePerBlobGas,
});
}
} else {
errorMsg = await l1TxUtils.tryGetErrorFromRevertedTx(data, args);
}

return { receipt, gasPrice, errorMsg };
}
}
Expand Down
18 changes: 9 additions & 9 deletions yarn-project/ethereum/src/contracts/rollup.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { memoize } from '@aztec/foundation/decorators';
import { EthAddress } from '@aztec/foundation/eth-address';
import type { ViemSignature } from '@aztec/foundation/eth-signature';
import { createLogger } from '@aztec/foundation/log';
import { RollupAbi, SlasherAbi } from '@aztec/l1-artifacts';

import {
Expand Down Expand Up @@ -35,9 +34,16 @@ export type L1RollupContractAddresses = Pick<
| 'rewardDistributorAddress'
>;

export type EpochProofQuoteViemArgs = {
epochToProve: bigint;
validUntilSlot: bigint;
bondAmount: bigint;
prover: `0x${string}`;
basisPointFee: number;
};

export class RollupContract {
private readonly rollup: GetContractReturnType<typeof RollupAbi, PublicClient<HttpTransport, Chain>>;
private readonly log = createLogger('rollup');

static getFromL1ContractsValues(deployL1ContractsValues: DeployL1Contracts) {
const {
Expand Down Expand Up @@ -266,13 +272,7 @@ export class RollupContract {

public async validateProofQuote(
quote: {
quote: {
epochToProve: bigint;
validUntilSlot: bigint;
bondAmount: bigint;
prover: `0x${string}`;
basisPointFee: number;
};
quote: EpochProofQuoteViemArgs;
signature: ViemSignature;
},
account: `0x${string}` | Account,
Expand Down
28 changes: 16 additions & 12 deletions yarn-project/ethereum/src/l1_tx_utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -638,13 +638,8 @@ export class L1TxUtils {
abi: Abi;
address: Hex;
},
_blobInputs?: {
blobs: Uint8Array[];
kzg: any;
maxFeePerBlobGas?: bigint;
},
blobInputs?: L1BlobInputs & { maxFeePerBlobGas: bigint },
) {
const blobInputs = _blobInputs || {};
try {
// NB: If this fn starts unexpectedly giving incorrect blob hash errors, it may be because the checkBlob
// bool is no longer at the slot below. To find the slot, run: forge inspect src/core/Rollup.sol:Rollup storage
Expand All @@ -668,12 +663,21 @@ export class L1TxUtils {
// Strangely, the only way to throw the revert reason as an error and provide blobs is prepareTransactionRequest.
// See: https://github.com/wevm/viem/issues/2075
// This throws a EstimateGasExecutionError with the custom error information:
await this.walletClient.prepareTransactionRequest({
account: this.walletClient.account,
to: args.address,
data,
...blobInputs,
});
const request = blobInputs
? {
account: this.walletClient.account,
to: args.address,
data,
blobs: blobInputs.blobs,
kzg: blobInputs.kzg,
maxFeePerBlobGas: blobInputs.maxFeePerBlobGas,
}
: {
account: this.walletClient.account,
to: args.address,
data,
};
await this.walletClient.prepareTransactionRequest(request);
return undefined;
} catch (simulationErr: any) {
// If we don't have a ContractFunctionExecutionError, we have a blob related error => use getContractError to get the error msg.
Expand Down

0 comments on commit 00da5ff

Please sign in to comment.