Skip to content

Commit

Permalink
remove reliance on fork testing and improve Prover contract security
Browse files Browse the repository at this point in the history
  • Loading branch information
@jackchuma
jackchuma committed Nov 18, 2024
1 parent 66c157d commit 51496e3
Show file tree
Hide file tree
Showing 12 changed files with 368 additions and 190 deletions.
4 changes: 2 additions & 2 deletions contracts/Makefile
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,11 @@

test:
forge fmt
forge test --fork-url https://sepolia.base.org --fork-block-number 18070252
forge test

coverage:
forge fmt
forge coverage --fork-url https://sepolia.base.org --fork-block-number 18070252
forge coverage

deploy-mock:
forge create --rpc-url $(ARBITRUM_SEPOLIA_RPC) --private-key $(PRIVATE_KEY) test/mocks/MockVerifier.sol:MockVerifier
Expand Down
21 changes: 11 additions & 10 deletions contracts/src/provers/ArbitrumProver.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,10 +77,6 @@ contract ArbitrumProver is IProver {
CrossChainRequest calldata request,
bytes calldata proof
) external view {
if (block.timestamp - fulfillmentInfo.timestamp < request.finalityDelaySeconds) {
revert FinalityDelaySecondsInProgress();
}

RIP7755Proof memory proofData = abi.decode(proof, (RIP7755Proof));

// Set the expected storage key and value for the `RIP7755Inbox` on Arbitrum
Expand Down Expand Up @@ -113,8 +109,13 @@ contract ArbitrumProver is IProver {
bytes32 l2BlockHash = keccak256(proofData.encodedBlockArray);
// Derive the RBlock's `confirmData` field
bytes32 confirmData = keccak256(abi.encodePacked(l2BlockHash, proofData.sendRoot));
// Extract the L2 stateRoot from the RLP-encoded block array
bytes32 l2StateRoot = _extractL2StateRoot(proofData.encodedBlockArray);
// Extract the L2 stateRoot and timestamp from the RLP-encoded block array
(bytes32 l2StateRoot, uint256 l2Timestamp) = _extractL2StateRootAndTimestamp(proofData.encodedBlockArray);

// Ensure that the fulfillment timestamp is not within the finality delay
if (fulfillmentInfo.timestamp + request.finalityDelaySeconds > l2Timestamp) {
revert FinalityDelaySecondsInProgress();
}

// The L1 storage value we proved was the node's confirmData
if (bytes32(proofData.dstL2StateRootProofParams.storageValue) != confirmData) {
Expand Down Expand Up @@ -144,19 +145,19 @@ contract ArbitrumProver is IProver {
return abi.encodePacked(startingStorageSlot + _ARBITRUM_RBLOCK_CONFIRMDATA_STORAGE_OFFSET);
}

/// @notice Extracts the l2StateRoot from the RLP-encoded block headers array
/// @notice Extracts the l2StateRoot and l2Timestamp from the RLP-encoded block headers array
///
/// @custom:reverts If the encoded block array does not have 16 elements
///
/// @dev The stateRoot should be the fourth element
function _extractL2StateRoot(bytes memory encodedBlockArray) private pure returns (bytes32) {
/// @dev The stateRoot should be the fourth element, and the timestamp should be the eleventh element
function _extractL2StateRootAndTimestamp(bytes memory encodedBlockArray) private pure returns (bytes32, uint256) {
RLPReader.RLPItem[] memory blockFields = encodedBlockArray.readList();

if (blockFields.length != 16) {
revert InvalidBlockFieldRLP();
}

return bytes32(blockFields[3].readBytes()); // state root is the fourth field
return (bytes32(blockFields[3].readBytes()), uint256(bytes32(blockFields[11].readBytes())));
}

/// @dev Encodes the FulfillmentInfo struct the way it should be stored on the destination chain
Expand Down
46 changes: 36 additions & 10 deletions contracts/src/provers/OPStackProver.sol
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
// SPDX-License-Identifier: MIT
pragma solidity 0.8.24;

import {RLPReader} from "optimism/packages/contracts-bedrock/src/libraries/rlp/RLPReader.sol";

import {IProver} from "../interfaces/IProver.sol";
import {StateValidator} from "../libraries/StateValidator.sol";
import {RIP7755Inbox} from "../RIP7755Inbox.sol";
Expand All @@ -13,15 +15,15 @@ import {CrossChainRequest} from "../RIP7755Structs.sol";
/// @notice This contract implements storage proof validation to ensure that requested calls actually happened on an OP Stack chain
contract OPStackProver is IProver {
using StateValidator for address;
using RLPReader for RLPReader.RLPItem;
using RLPReader for bytes;

/// @notice Parameters needed for a full nested cross-L2 storage proof
struct RIP7755Proof {
/// @dev The L2 stateRoot used to prove L2 storage value in `RIP7755Inbox`
bytes32 l2StateRoot;
/// @dev The storage root of Optimism's MessagePasser contract - used to compute our L1 storage value
bytes32 l2MessagePasserStorageRoot;
/// @dev the blockhash of the L2 block corresponding to the above l2StateRoot
bytes32 l2BlockHash;
/// @dev The RLP-encoded array of block headers of the chain's L2 block used for the proof. Hashing this bytes string should produce the blockhash.
bytes encodedBlockArray;
/// @dev Parameters needed to validate the authenticity of Ethereum's execution client's state root
StateValidator.StateProofParameters stateProofParams;
/// @dev Parameters needed to validate the authenticity of the l2Oracle for the destination L2 chain on Eth
Expand All @@ -47,6 +49,9 @@ contract OPStackProver is IProver {
/// @notice This error is thrown when the supplied l2StateRoot does not correspond to our validated L1 state
error InvalidL2StateRoot();

/// @notice This error is thrown when the encoded block headers does not contain all 16 fields
error InvalidBlockFieldRLP();

/// @notice Validates storage proofs and verifies fulfillment
///
/// @custom:reverts If storage proof invalid.
Expand All @@ -69,10 +74,6 @@ contract OPStackProver is IProver {
CrossChainRequest calldata request,
bytes calldata proof
) external view {
if (block.timestamp - fulfillmentInfo.timestamp < request.finalityDelaySeconds) {
revert FinalityDelaySecondsInProgress();
}

RIP7755Proof memory proofData = abi.decode(proof, (RIP7755Proof));

// Set the expected storage key and value for the `RIP7755Inbox` on the destination OP Stack chain
Expand All @@ -97,10 +98,20 @@ contract OPStackProver is IProver {
// to the correct l2StateRoot before we can prove l2Storage

bytes32 version;
// Extract the L2 stateRoot and timestamp from the RLP-encoded block array
(bytes32 l2StateRoot, uint256 l2Timestamp) = _extractL2StateRootAndTimestamp(proofData.encodedBlockArray);
// Derive the L2 blockhash
bytes32 l2BlockHash = keccak256(proofData.encodedBlockArray);

// Ensure that the fulfillment timestamp is not within the finality delay
if (fulfillmentInfo.timestamp + request.finalityDelaySeconds > l2Timestamp) {
revert FinalityDelaySecondsInProgress();
}

// Compute the expected destination chain output root (which is the value we just proved is in the L1 storage slot)
bytes32 expectedOutputRoot = keccak256(
abi.encodePacked(
version, proofData.l2StateRoot, proofData.l2MessagePasserStorageRoot, proofData.l2BlockHash
version, l2StateRoot, proofData.l2MessagePasserStorageRoot, l2BlockHash
)
);
// If this checks out, it means we know the correct l2StateRoot
Expand All @@ -115,7 +126,7 @@ contract OPStackProver is IProver {
// 6. Validate storage proof proving FulfillmentInfo in `RIP7755Inbox` storage
// NOTE: the following line is a temporary line used to validate proof logic. Will be removed in the near future.
bool validL2Storage = 0xAd6A7addf807D846A590E76C5830B609F831Ba2E.validateAccountStorage(
proofData.l2StateRoot, proofData.dstL2AccountProofParams
l2StateRoot, proofData.dstL2AccountProofParams
);
// bool validL2Storage =
// request.inboxContract.validateAccountStorage(proofData.l2StateRoot, proofData.dstL2AccountProofParams);
Expand All @@ -125,6 +136,21 @@ contract OPStackProver is IProver {
}
}

/// @notice Extracts the l2StateRoot and l2Timestamp from the RLP-encoded block headers array
///
/// @custom:reverts If the encoded block array does not have 16 elements
///
/// @dev The stateRoot should be the fourth element, and the timestamp should be the eleventh element
function _extractL2StateRootAndTimestamp(bytes memory encodedBlockArray) private pure returns (bytes32, uint256) {
RLPReader.RLPItem[] memory blockFields = encodedBlockArray.readList();

if (blockFields.length < 15) {
revert InvalidBlockFieldRLP();
}

return (bytes32(blockFields[3].readBytes()), uint256(bytes32(blockFields[11].readBytes())));
}

/// @dev Encodes the FulfillmentInfo struct the way it should be stored on the destination chain
function _encodeFulfillmentInfo(RIP7755Inbox.FulfillmentInfo calldata fulfillmentInfo)
internal
Expand Down
41 changes: 24 additions & 17 deletions contracts/test/ArbitrumProver.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,14 @@ import {StateValidator} from "../src/libraries/StateValidator.sol";
import {ArbitrumProver} from "../src/provers/ArbitrumProver.sol";
import {RIP7755Inbox} from "../src/RIP7755Inbox.sol";
import {Call, CrossChainRequest} from "../src/RIP7755Structs.sol";
import {MockBeaconOracle} from "./mocks/MockBeaconOracle.sol";

contract ArbitrumProverTest is Test {
using stdJson for string;

ArbitrumProver prover;
ERC20Mock mockErc20;
MockBeaconOracle mockBeaconOracle;

Call[] calls;
address ALICE = makeAddr("alice");
Expand All @@ -25,6 +27,7 @@ contract ArbitrumProverTest is Test {
string invalidL1State;
string invalidConfirmData;
string invalidBlockHeaders;
string finalityDelayInProgress;
uint256 private _REWARD_AMOUNT = 1 ether;
bytes32 private constant _VERIFIER_STORAGE_LOCATION =
0x43f1016e17bdb0194ec37b77cf476d255de00011d02616ab831d2e2ce63d9ee2;
Expand All @@ -33,6 +36,8 @@ contract ArbitrumProverTest is Test {
DeployArbitrumProver deployer = new DeployArbitrumProver();
prover = deployer.run();
mockErc20 = new ERC20Mock();
deployCodeTo("MockBeaconOracle.sol", abi.encode(), 0x000F3df6D732807Ef1319fB7B8bB8522d0Beac02);
mockBeaconOracle = MockBeaconOracle(0x000F3df6D732807Ef1319fB7B8bB8522d0Beac02);

string memory rootPath = vm.projectRoot();
string memory path = string.concat(rootPath, "/test/data/ArbitrumSepoliaProof.json");
Expand All @@ -41,10 +46,13 @@ contract ArbitrumProverTest is Test {
string.concat(rootPath, "/test/data/invalids/ArbitrumInvalidConfirmData.json");
string memory invalidBlockHeadersPath =
string.concat(rootPath, "/test/data/invalids/ArbitrumInvalidBlockHeaders.json");
string memory finalityDelayInProgressPath =
string.concat(rootPath, "/test/data/invalids/ArbitrumFinalityDelayInProgress.json");
validProof = vm.readFile(path);
invalidL1State = vm.readFile(invalidPath);
invalidConfirmData = vm.readFile(invalidConfirmDataPath);
invalidBlockHeaders = vm.readFile(invalidBlockHeadersPath);
finalityDelayInProgress = vm.readFile(finalityDelayInProgressPath);
}

modifier fundAlice(uint256 amount) {
Expand All @@ -57,74 +65,72 @@ contract ArbitrumProverTest is Test {

function test_reverts_ifFinalityDelaySecondsStillInProgress() external fundAlice(_REWARD_AMOUNT) {
CrossChainRequest memory request = _initRequest(_REWARD_AMOUNT);
request.finalityDelaySeconds = 1 ether;
request.expiry = 2 ether;

RIP7755Inbox.FulfillmentInfo memory fillInfo = _initFulfillmentInfo();
bytes memory storageProofData = _buildProof(validProof);
ArbitrumProver.RIP7755Proof memory proof = _buildProof(finalityDelayInProgress);
bytes memory inboxStorageKey = _deriveStorageKey(request);

vm.prank(FILLER);
vm.expectRevert(ArbitrumProver.FinalityDelaySecondsInProgress.selector);
prover.validateProof(inboxStorageKey, fillInfo, request, storageProofData);
prover.validateProof(inboxStorageKey, fillInfo, request, abi.encode(proof));
}

function test_reverts_ifInvalidL1State() external fundAlice(_REWARD_AMOUNT) {
CrossChainRequest memory request = _initRequest(_REWARD_AMOUNT);
RIP7755Inbox.FulfillmentInfo memory fillInfo = _initFulfillmentInfo();
bytes memory storageProofData = _buildProof(invalidL1State);
ArbitrumProver.RIP7755Proof memory proof = _buildProof(invalidL1State);
bytes memory inboxStorageKey = _deriveStorageKey(request);

vm.prank(FILLER);
vm.expectRevert(ArbitrumProver.InvalidStateRoot.selector);
prover.validateProof(inboxStorageKey, fillInfo, request, storageProofData);
prover.validateProof(inboxStorageKey, fillInfo, request, abi.encode(proof));
}

function test_reverts_ifInvalidRLPHeaders() external fundAlice(_REWARD_AMOUNT) {
CrossChainRequest memory request = _initRequest(_REWARD_AMOUNT);
RIP7755Inbox.FulfillmentInfo memory fillInfo = _initFulfillmentInfo();
bytes memory storageProofData = _buildProof(invalidBlockHeaders);
ArbitrumProver.RIP7755Proof memory proof = _buildProof(invalidBlockHeaders);
bytes memory inboxStorageKey = _deriveStorageKey(request);

vm.prank(FILLER);
vm.expectRevert(ArbitrumProver.InvalidBlockFieldRLP.selector);
prover.validateProof(inboxStorageKey, fillInfo, request, storageProofData);
prover.validateProof(inboxStorageKey, fillInfo, request, abi.encode(proof));
}

function test_reverts_ifInvalidConfirmData() external fundAlice(_REWARD_AMOUNT) {
CrossChainRequest memory request = _initRequest(_REWARD_AMOUNT);
RIP7755Inbox.FulfillmentInfo memory fillInfo = _initFulfillmentInfo();
bytes memory storageProofData = _buildProof(invalidConfirmData);
ArbitrumProver.RIP7755Proof memory proof = _buildProof(invalidConfirmData);
bytes memory inboxStorageKey = _deriveStorageKey(request);

vm.prank(FILLER);
vm.expectRevert(ArbitrumProver.InvalidConfirmData.selector);
prover.validateProof(inboxStorageKey, fillInfo, request, storageProofData);
prover.validateProof(inboxStorageKey, fillInfo, request, abi.encode(proof));
}

function test_reverts_ifInvalidL2Storage() external fundAlice(_REWARD_AMOUNT) {
CrossChainRequest memory request = _initRequest(_REWARD_AMOUNT);
RIP7755Inbox.FulfillmentInfo memory fillInfo = _initFulfillmentInfo();
fillInfo.timestamp++;
bytes memory storageProofData = _buildProof(validProof);
ArbitrumProver.RIP7755Proof memory proof = _buildProof(validProof);
bytes memory inboxStorageKey = _deriveStorageKey(request);

vm.prank(FILLER);
vm.expectRevert(ArbitrumProver.InvalidL2Storage.selector);
prover.validateProof(inboxStorageKey, fillInfo, request, storageProofData);
prover.validateProof(inboxStorageKey, fillInfo, request, abi.encode(proof));
}

function test_proveArbitrumSepoliaStateFromBaseSepolia() external fundAlice(_REWARD_AMOUNT) {
CrossChainRequest memory request = _initRequest(_REWARD_AMOUNT);
RIP7755Inbox.FulfillmentInfo memory fillInfo = _initFulfillmentInfo();
bytes memory storageProofData = _buildProof(validProof);
ArbitrumProver.RIP7755Proof memory proof = _buildProof(validProof);
bytes memory inboxStorageKey = _deriveStorageKey(request);

vm.prank(FILLER);
prover.validateProof(inboxStorageKey, fillInfo, request, storageProofData);
prover.validateProof(inboxStorageKey, fillInfo, request, abi.encode(proof));
}

function _buildProof(string memory json) private pure returns (bytes memory) {
function _buildProof(string memory json) private returns (ArbitrumProver.RIP7755Proof memory) {
StateValidator.StateProofParameters memory stateProofParams = StateValidator.StateProofParameters({
beaconRoot: json.readBytes32(".stateProofParams.beaconRoot"),
beaconOracleTimestamp: uint256(json.readBytes32(".stateProofParams.beaconOracleTimestamp")),
Expand All @@ -144,15 +150,16 @@ contract ArbitrumProverTest is Test {
storageProof: abi.decode(json.parseRaw(".dstL2AccountProofParams.storageProof"), (bytes[]))
});

ArbitrumProver.RIP7755Proof memory proofData = ArbitrumProver.RIP7755Proof({
mockBeaconOracle.commitBeaconRoot(1, stateProofParams.beaconOracleTimestamp, stateProofParams.beaconRoot);

return ArbitrumProver.RIP7755Proof({
sendRoot: json.readBytes(".sendRoot"),
encodedBlockArray: json.readBytes(".encodedBlockArray"),
stateProofParams: stateProofParams,
dstL2StateRootProofParams: dstL2StateRootParams,
dstL2AccountProofParams: dstL2AccountProofParams,
nodeIndex: uint64(json.readUint(".nodeIndex"))
});
return abi.encode(proofData);
}

function _initRequest(uint256 rewardAmount) private view returns (CrossChainRequest memory) {
Expand Down
Loading

0 comments on commit 51496e3

Please sign in to comment.