Skip to content
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

feat(contract): implement FraudProofRouter #4927

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Open
Changes from 8 commits
Commits
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
24 changes: 9 additions & 15 deletions solidity/contracts/AttributeCheckpointFraud.sol
Original file line number Diff line number Diff line change
@@ -5,29 +5,16 @@ import {ECDSA} from "@openzeppelin/contracts/utils/cryptography/ECDSA.sol";
import {Address} from "@openzeppelin/contracts/utils/Address.sol";
import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol";

import {PackageVersioned} from "contracts/PackageVersioned.sol";
import {PackageVersioned} from "./PackageVersioned.sol";
import {TREE_DEPTH} from "./libs/Merkle.sol";
import {CheckpointLib, Checkpoint} from "./libs/CheckpointLib.sol";
import {CheckpointFraudProofs} from "./CheckpointFraudProofs.sol";

enum FraudType {
Whitelist,
Premature,
MessageId,
Root
}

struct Attribution {
FraudType fraudType;
// for comparison with staking epoch
uint48 timestamp;
}
import {FraudType, Attribution} from "./libs/FraudMessage.sol";
aroralanuk marked this conversation as resolved.
Show resolved Hide resolved

/**
* @title AttributeCheckpointFraud
* @dev The AttributeCheckpointFraud contract is used to attribute fraud to a specific ECDSA checkpoint signer.
*/

contract AttributeCheckpointFraud is Ownable, PackageVersioned {
using CheckpointLib for Checkpoint;
using Address for address;
@@ -72,6 +59,13 @@ contract AttributeCheckpointFraud is Ownable, PackageVersioned {
return _attributions[signer][digest];
}

function attributions(
address signer,
bytes32 digest
) external view returns (Attribution memory) {
return _attributions[signer][digest];
}

function whitelist(address merkleTree) external onlyOwner {
require(
merkleTree.isContract(),
59 changes: 59 additions & 0 deletions solidity/contracts/libs/FraudMessage.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
// SPDX-License-Identifier: MIT OR Apache-2.0
pragma solidity >=0.8.0;

/*@@@@@@@ @@@@@@@@@
@@@@@@@@@ @@@@@@@@@
@@@@@@@@@ @@@@@@@@@
@@@@@@@@@ @@@@@@@@@
@@@@@@@@@@@@@@@@@@@@@@@@@
@@@@@ HYPERLANE @@@@@@@
@@@@@@@@@@@@@@@@@@@@@@@@@
@@@@@@@@@ @@@@@@@@@
@@@@@@@@@ @@@@@@@@@
@@@@@@@@@ @@@@@@@@@
@@@@@@@@@ @@@@@@@@*/

enum FraudType {
Whitelist,
Premature,
MessageId,
Root
}

struct Attribution {
FraudType fraudType;
// for comparison with staking epoch
uint48 timestamp;
}

library FraudMessage {
function encode(
address signer,
bytes32 merkleTree,
bytes32 digest,
Attribution memory attribution
) internal pure returns (bytes memory) {
return
abi.encodePacked(
signer,
merkleTree,
digest,
uint8(attribution.fraudType),
attribution.timestamp
);
}

function decode(
bytes calldata _message
) internal pure returns (address, bytes32, bytes32, Attribution memory) {
require(_message.length == 91, "Invalid message length");

address signer = address(bytes20(_message[0:20]));
bytes32 merkleTree = bytes32(_message[20:52]);
aroralanuk marked this conversation as resolved.
Show resolved Hide resolved
bytes32 digest = bytes32(_message[52:84]);
FraudType fraudType = FraudType(uint8(_message[84]));
uint48 timestamp = uint48(bytes6(_message[85:91]));

return (signer, merkleTree, digest, Attribution(fraudType, timestamp));
}
}
128 changes: 128 additions & 0 deletions solidity/contracts/middleware/FraudProofRouter.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,128 @@
// SPDX-License-Identifier: MIT OR Apache-2.0
pragma solidity >=0.8.0;
Fixed Show fixed Hide fixed

Check notice

Code scanning / Olympix Integrated Security

Using an unbounded pragma for Solidity version may be unsafe if future versions introduce breaking changes. For more information, visit: http://detectors.olympixdevsectools.com/article/web3-vulnerability/unbounded-pragma Low

Using an unbounded pragma for Solidity version may be unsafe if future versions introduce breaking changes. For more information, visit: http://detectors.olympixdevsectools.com/article/web3-vulnerability/unbounded-pragma

/*@@@@@@@ @@@@@@@@@
@@@@@@@@@ @@@@@@@@@
@@@@@@@@@ @@@@@@@@@
@@@@@@@@@ @@@@@@@@@
@@@@@@@@@@@@@@@@@@@@@@@@@
@@@@@ HYPERLANE @@@@@@@
@@@@@@@@@@@@@@@@@@@@@@@@@
@@@@@@@@@ @@@@@@@@@
@@@@@@@@@ @@@@@@@@@
@@@@@@@@@ @@@@@@@@@
@@@@@@@@@ @@@@@@@@*/

import {TypeCasts} from "../libs/TypeCasts.sol";
import {FraudType, FraudMessage, Attribution} from "../libs/FraudMessage.sol";
import {AttributeCheckpointFraud} from "../AttributeCheckpointFraud.sol";
import {GasRouter} from "../client/GasRouter.sol";

contract FraudProofRouter is GasRouter {
// ===================== State Variables =======================

// The AttributeCheckpointFraud contract to obtain the attributions from
AttributeCheckpointFraud public immutable attributeCheckpointFraud;
Dismissed Show dismissed Hide dismissed

// Mapping to store the fraud attributions for a given origin, signer, merkle tree, and digest for easy access for client contracts to aide slashing
mapping(uint32 origin => mapping(address signer => mapping(bytes32 merkleTree => mapping(bytes32 digest => Attribution))))
Fixed Show fixed Hide fixed
public fraudAttributions;

// ===================== Events =======================

event FraudProofSent(
Fixed Show fixed Hide fixed
address indexed signer,
bytes32 indexed digest,
Attribution attribution
);

event FraudProofReceived(
Fixed Show fixed Hide fixed
uint32 indexed origin,
address indexed signer,
bytes32 indexed digest,
Attribution attribution
);

// ===================== Constructor =======================

/**
* @notice Initializes the FraudProofRouter with the mailbox address and AttributeCheckpointFraud contract.
* @param _mailbox The address of the mailbox contract.
* @param _attributeCheckpointFraud The address of the AttributeCheckpointFraud contract.
*/
constructor(

Check notice

Code scanning / Olympix Integrated Security

Test functions fail to thoroughly test all aspects of contract constructors, potentially missing critical initialization issues. For more information, visit: http://detectors.olympixdevsectools.com/article/web3-vulnerability/incomplete-constructor-tests Low

Test functions fail to thoroughly test all aspects of contract constructors, potentially missing critical initialization issues. For more information, visit: http://detectors.olympixdevsectools.com/article/web3-vulnerability/incomplete-constructor-tests
address _mailbox,
Dismissed Show dismissed Hide dismissed
address _attributeCheckpointFraud
) GasRouter(_mailbox) {
require(

Check warning

Code scanning / Olympix Integrated Security

Test functions fail to verify specific revert reasons, potentially missing important contract behavior validation. For more information, visit: http://detectors.olympixdevsectools.com/article/web3-vulnerability/missing-revert-reason-tests Medium

Test functions fail to verify specific revert reasons, potentially missing important contract behavior validation. For more information, visit: http://detectors.olympixdevsectools.com/article/web3-vulnerability/missing-revert-reason-tests
_attributeCheckpointFraud != address(0),
"Invalid AttributeCheckpointFraud address"
);
attributeCheckpointFraud = AttributeCheckpointFraud(
_attributeCheckpointFraud
);
hook = mailbox.defaultHook();
}

/**
* @notice Sends a fraud proof attribution.
* @param _signer The address of the signer attributed with fraud.
* @param _digest The digest associated with the fraud.
* @param _merkleTree The merkle tree associated with the fraud.
* @return The message ID of the sent fraud proof.
*/
function sendFraudProof(
Fixed Show fixed Hide fixed
Fixed Show fixed Hide fixed
Fixed Show fixed Hide fixed

Check failure

Code scanning / Olympix Integrated Security

Modifying state after making an external call may allow for reentrancy attacks. For more information, visit: http://detectors.olympixdevsectools.com/article/web3-vulnerability/reentrancy Critical

Modifying state after making an external call may allow for reentrancy attacks. For more information, visit: http://detectors.olympixdevsectools.com/article/web3-vulnerability/reentrancy

Check notice

Code scanning / Olympix Integrated Security

Reentrant functions which emit events after making an external call may lead to out-of-order events. For more information, visit: http://detectors.olympixdevsectools.com/article/web3-vulnerability/reentrancy-events Low

Reentrant functions which emit events after making an external call may lead to out-of-order events. For more information, visit: http://detectors.olympixdevsectools.com/article/web3-vulnerability/reentrancy-events
uint32 _destination,
address _signer,
bytes32 _merkleTree,
bytes32 _digest
) external returns (bytes32) {
Attribution memory attribution = attributeCheckpointFraud.attributions(
_signer,
_digest
);
Comment on lines +84 to +87
Copy link
Member

Choose a reason for hiding this comment

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

I thought you would literally merge these two contracts
as it stands the storage is still redundant

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

I did, but you still have 2 mappings

Copy link
Member

Choose a reason for hiding this comment

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

the AttributeCheckpointFraud.attributions mapping has redundant information with this contract's fraudAttributions[chainId]
I am proposing merging these contracts, having a single mapping, and a single storage location for this information


require(attribution.timestamp != 0, "Attribution does not exist");
Fixed Show fixed Hide fixed
Fixed Show fixed Hide fixed

Check warning

Code scanning / Olympix Integrated Security

Test functions fail to verify specific revert reasons, potentially missing important contract behavior validation. For more information, visit: http://detectors.olympixdevsectools.com/article/web3-vulnerability/missing-revert-reason-tests Medium

Test functions fail to verify specific revert reasons, potentially missing important contract behavior validation. For more information, visit: http://detectors.olympixdevsectools.com/article/web3-vulnerability/missing-revert-reason-tests

bytes memory encodedMessage = FraudMessage.encode(
_signer,
_merkleTree,
_digest,
attribution
);

emit FraudProofSent(_signer, _digest, attribution);

return
_Router_dispatch(
_destination,
0,
encodedMessage,
"",
address(hook)
);
}

/**
* @notice Handles by decoding the inbound fraud proof message.
* @param _origin The origin domain of the fraud proof.
* @param _message The encoded fraud proof message.
*/
function _handle(
uint32 _origin,
bytes32,
/*_sender*/
bytes calldata _message
) internal override {
(
address signer,
Fixed Show fixed Hide fixed

Check notice

Code scanning / Olympix Integrated Security

Local variables in test functions are not properly fuzzed, potentially reducing the effectiveness of property-based testing. For more information, visit: http://detectors.olympixdevsectools.com/article/web3-vulnerability/unfuzzed-local-variables Low

Local variables in test functions are not properly fuzzed, potentially reducing the effectiveness of property-based testing. For more information, visit: http://detectors.olympixdevsectools.com/article/web3-vulnerability/unfuzzed-local-variables
bytes32 merkleTree,
Fixed Show fixed Hide fixed
bytes32 digest,

Check notice

Code scanning / Olympix Integrated Security

Local variables in test functions are not properly fuzzed, potentially reducing the effectiveness of property-based testing. For more information, visit: http://detectors.olympixdevsectools.com/article/web3-vulnerability/unfuzzed-local-variables Low

Local variables in test functions are not properly fuzzed, potentially reducing the effectiveness of property-based testing. For more information, visit: http://detectors.olympixdevsectools.com/article/web3-vulnerability/unfuzzed-local-variables
Attribution memory attribution
Fixed Show fixed Hide fixed

Check notice

Code scanning / Olympix Integrated Security

Local variables in test functions are not properly fuzzed, potentially reducing the effectiveness of property-based testing. For more information, visit: http://detectors.olympixdevsectools.com/article/web3-vulnerability/unfuzzed-local-variables Low

Local variables in test functions are not properly fuzzed, potentially reducing the effectiveness of property-based testing. For more information, visit: http://detectors.olympixdevsectools.com/article/web3-vulnerability/unfuzzed-local-variables
) = FraudMessage.decode(_message);

fraudAttributions[_origin][signer][merkleTree][digest] = attribution;

emit FraudProofReceived(_origin, signer, digest, attribution);
}
}
20 changes: 20 additions & 0 deletions solidity/contracts/test/TestAttributeCheckpointFraud.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// SPDX-License-Identifier: MIT OR Apache-2.0
pragma solidity >=0.8.0;

import {FraudType, Attribution} from "../libs/FraudMessage.sol";
import {AttributeCheckpointFraud} from "../AttributeCheckpointFraud.sol";

contract TestAttributeCheckpointFraud is AttributeCheckpointFraud {
constructor() AttributeCheckpointFraud() {}
Dismissed Show dismissed Hide dismissed

function mockSetAttribution(
address signer,
bytes32 digest,
FraudType fraudType
) external {
_attributions[signer][digest] = Attribution({
fraudType: fraudType,
timestamp: uint48(block.timestamp)
});
}
}
132 changes: 132 additions & 0 deletions solidity/test/FraudProofRouter.t.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,132 @@
// SPDX-License-Identifier: MIT OR Apache-2.0
pragma solidity >=0.8.0;

import {Test} from "forge-std/Test.sol";

import {FraudType, Attribution} from "../contracts/libs/FraudMessage.sol";
import {TypeCasts} from "../contracts/libs/TypeCasts.sol";
import {TestAttributeCheckpointFraud} from "../contracts/test/TestAttributeCheckpointFraud.sol";
import {FraudProofRouter} from "../contracts/middleware/FraudProofRouter.sol";
import {MockMailbox} from "../contracts/mock/MockMailbox.sol";
import {TestMerkle} from "../contracts/test/TestMerkle.sol";

contract FraudProofRouterTest is Test {
using TypeCasts for address;

uint32 public constant LOCAL_DOMAIN = 1;
uint32 public constant DESTINATION_DOMAIN = 2;
MockMailbox internal localMailbox;
MockMailbox internal remoteMailbox;
TestMerkle internal testMerkleHook;
TestAttributeCheckpointFraud public testAcf;
FraudProofRouter public originFpr;
FraudProofRouter public remoteFpr;
address public constant OWNER = address(0x1);
address public constant SIGNER = address(0x2);
bytes32 DIGEST = keccak256(abi.encodePacked("digest"));

function setUp() public {
vm.warp(1000);
localMailbox = new MockMailbox(LOCAL_DOMAIN);
remoteMailbox = new MockMailbox(DESTINATION_DOMAIN);
localMailbox.addRemoteMailbox(DESTINATION_DOMAIN, remoteMailbox);
remoteMailbox.addRemoteMailbox(LOCAL_DOMAIN, localMailbox);

testMerkleHook = new TestMerkle();

testAcf = new TestAttributeCheckpointFraud();

vm.startPrank(OWNER);
originFpr = new FraudProofRouter(
address(localMailbox),
address(testAcf)
);
remoteFpr = new FraudProofRouter(
address(remoteMailbox),
address(testAcf)
);

originFpr.enrollRemoteRouter(
DESTINATION_DOMAIN,
address(remoteFpr).addressToBytes32()
);
remoteFpr.enrollRemoteRouter(
LOCAL_DOMAIN,
address(originFpr).addressToBytes32()
);

vm.stopPrank();
}

function test_setAttributeCheckpointFraud_invalidAddress() public {
vm.expectRevert("Invalid AttributeCheckpointFraud address");
new FraudProofRouter(address(localMailbox), address(0));
}

function test_sendFraudProof(
address _signer,
bytes32 _digest,
bytes32 _merkleTree,
uint8 _fraudType,
uint48 _timestamp
) public {
vm.assume(_fraudType <= uint8(FraudType.Root));
vm.assume(_timestamp > 0);
vm.warp(_timestamp);
FraudType fraudTypeEnum = FraudType(_fraudType);

testAcf.mockSetAttribution(_signer, _digest, fraudTypeEnum);

vm.expectEmit(true, true, true, true, address(originFpr));
emit FraudProofRouter.FraudProofSent(
_signer,
_digest,
Attribution(fraudTypeEnum, uint48(block.timestamp))
);

originFpr.sendFraudProof(
DESTINATION_DOMAIN,
_signer,
_merkleTree,
_digest
);

vm.expectEmit(true, true, true, true, address(remoteFpr));
emit FraudProofRouter.FraudProofReceived(
LOCAL_DOMAIN,
_signer,
_digest,
Attribution(fraudTypeEnum, uint48(block.timestamp))
);
remoteMailbox.processNextInboundMessage();

(FraudType actualFraudType, uint48 actualTimestamp) = remoteFpr
.fraudAttributions(LOCAL_DOMAIN, _signer, _merkleTree, _digest);

assert(actualFraudType == fraudTypeEnum);
assertEq(actualTimestamp, block.timestamp);
}

function test_sendFraudProof_noAttribution() public {
vm.expectRevert("Attribution does not exist");
originFpr.sendFraudProof(
DESTINATION_DOMAIN,
SIGNER,
TypeCasts.addressToBytes32(address(testMerkleHook)),
DIGEST
);
}

function test_sendFraudProof_routerNotEnrolled() public {
FraudType fraudType = FraudType.Whitelist;
testAcf.mockSetAttribution(SIGNER, DIGEST, fraudType);

vm.expectRevert("No router enrolled for domain: 3");
originFpr.sendFraudProof(
3,
SIGNER,
TypeCasts.addressToBytes32(address(testMerkleHook)),
DIGEST
);
}
}
Loading