Skip to content

Commit

Permalink
feat: separate deposit execution logic (#14)
Browse files Browse the repository at this point in the history 
* separate execution and deposit logic

* update test with deposit and execution logic changes

* Remove unused Migrations contract

* Fix variable name and comments

* Optimize reading domainID
  • Loading branch information
@nmlinaric
nmlinaric authored Dec 12, 2023
1 parent 3ca83b5 commit 4ea35ff
Show file tree
Hide file tree
Showing 47 changed files with 1,221 additions and 578 deletions.
2 changes: 1 addition & 1 deletion README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ Requires `yarn` and `@nomicfoundation/hardhat`.
`typechain` - Generate Typechain typings for compiled contracts <br>
`verify` - Verifies contract on Etherscan <br>
* custom commands: <br>
`yarn run test`: Runs truffle tests.
`yarn run test`: Runs tests.

# Sygma Security Policy

Expand Down
1 change: 1 addition & 0 deletions package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
"author": "Sygma",
"license": "BUSL-1.1",
"scripts": {
"compile": "npx hardhat compile",
"coverage": "npx hardhat coverage",
"lint:typescript": "eslint 'test/**/*.ts' 'scripts/**/*.ts'",
"lint:typescript:fix": "prettier --config .prettierrc.json --write 'scripts/**/*.ts' 'test/**/*.ts' 'hardhat.config.ts'",
Expand Down
110 changes: 60 additions & 50 deletions scripts/utils.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,57 +3,67 @@ import { resolve } from "path";
import type { InterfaceAbi, Fragment } from "ethers";
import { keccak256, ContractFactory } from "ethers";

const BRIDGE_CONTRACT_PATH = resolve(__dirname, "../src/contracts/Bridge.sol");
const ARTIFACTS_PATH = resolve(
__dirname,
"../artifacts/src/contracts/Bridge.sol/Bridge.json",
);

export function generateAccessControlFuncSignatures(): {
export function generateAccessControlFuncSignatures(contracts: Array<string>): {
function: string;
hash: string;
}[] {
const bridgeArtifacts = JSON.parse(
fs.readFileSync(ARTIFACTS_PATH).toString(),
);
const bridgeContractFactory = new ContractFactory(
bridgeArtifacts.abi as InterfaceAbi,
bridgeArtifacts.bytecode as string,
);
const bridgeContractMethods = bridgeContractFactory.interface.fragments
.map((fragment: Fragment) => {
if (fragment.type == "function") {
return fragment.format();
}
})
.filter((item) => item) as Array<string>;

const bridgeContract = fs.readFileSync(BRIDGE_CONTRACT_PATH);

// regex that will match all functions that have "onlyAllowed" modifier
const regex = RegExp(
"function\\s+(?:(?!_onlyAllowed|function).)+onlyAllowed",
"gs",
);

let a;
const b: Array<string> = [];
// fetch all functions that have "onlyAllowed" modifier from "Bridge.sol"
while ((a = regex.exec(bridgeContract.toString())) !== null) {
// filter out only function name from matching (onlyAllowed) functions
b.push(a[0].split(/[\s()]+/)[1]);
}

let accessControlFuncSignatures = [];
// filter out from Bridge ABI functions signatures with "onlyAllowed" modifier
accessControlFuncSignatures = bridgeContractMethods
.filter((el1) => b.some((el2) => el1.includes(el2)))
.map((func) => ({
function: func,
hash: keccak256(Buffer.from(func)).substring(0, 10),
}));

console.table(accessControlFuncSignatures);

return accessControlFuncSignatures;
const allAccessControlFuncSignatures: {
function: string;
hash: string;
}[] = [];

contracts.map((contractName) => {
const CONTRACT_PATH = resolve(
__dirname,
`../src/contracts/${contractName}.sol`,
);
const ARTIFACTS_PATH = resolve(
__dirname,
`../artifacts/src/contracts/${contractName}.sol/${contractName}.json`,
);

const bridgeArtifacts = JSON.parse(
fs.readFileSync(ARTIFACTS_PATH).toString(),
);
const contractFactory = new ContractFactory(
bridgeArtifacts.abi as InterfaceAbi,
bridgeArtifacts.bytecode as string,
);
const contractMethods = contractFactory.interface.fragments
.map((fragment: Fragment) => {
if (fragment.type == "function") {
return fragment.format();
}
})
.filter((item) => item) as Array<string>;

const contractInstance = fs.readFileSync(CONTRACT_PATH);

// regex that will match all functions that have "onlyAllowed" modifier
const regex = RegExp(
"function\\s+(?:(?!_onlyAllowed|function).)+onlyAllowed",
"gs",
);

let a;
const b: Array<string> = [];
// fetch all functions that have "onlyAllowed" modifier
while ((a = regex.exec(contractInstance.toString())) !== null) {
// filter out only function name from matching (onlyAllowed) functions
b.push(a[0].split(/[\s()]+/)[1]);
}

// filter out from Bridge ABI functions signatures with "onlyAllowed" modifier
const accessControlFuncSignatures = contractMethods
.filter((el1) => b.some((el2) => el1.includes(el2)))
.map((func) => ({
function: func,
hash: keccak256(Buffer.from(func)).substring(0, 10),
}));
allAccessControlFuncSignatures.push(...accessControlFuncSignatures);
});

console.table(allAccessControlFuncSignatures);

return allAccessControlFuncSignatures;
}
194 changes: 1 addition & 193 deletions src/contracts/Bridge.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,54 +25,16 @@ contract Bridge is Pausable, Context {

IAccessControlSegregator public _accessControl;

struct Proposal {
uint8 originDomainID;
uint64 depositNonce;
bytes32 resourceID;
bytes data;
}

// destinationDomainID => number of deposits
mapping(uint8 => uint64) public _depositCounts;
// resourceID => handler address
mapping(bytes32 => address) public _resourceIDToHandlerAddress;
// forwarder address => is Valid
mapping(address => bool) public isValidForwarder;
// origin domainID => nonces set => used deposit nonces
mapping(uint8 => mapping(uint256 => uint256)) public usedNonces;


event FeeHandlerChanged(address newFeeHandler);
event AccessControlChanged(address newAccessControl);
event Deposit(
uint8 destinationDomainID,
bytes32 resourceID,
uint64 depositNonce,
address indexed user,
bytes data,
bytes handlerResponse
);
event ProposalExecution(uint8 originDomainID, uint64 depositNonce, bytes32 dataHash, bytes handlerResponse);

event FailedHandlerExecution(bytes lowLevelData, uint8 originDomainID, uint64 depositNonce);

event StartKeygen();

event EndKeygen();

event KeyRefresh(string hash);

event Retry(string txHash);

error AccessNotAllowed(address sender, bytes4 funcSig);

error ResourceIDNotMappedToHandler();

error DepositToCurrentDomain();

error EmptyProposalsArray();

error NonceDecrementsNotAllowed();

modifier onlyAllowed() {
_onlyAllowed(msg.sig, _msgSender());
_;
Expand All @@ -82,16 +44,6 @@ contract Bridge is Pausable, Context {
if (!_accessControl.hasAccess(sig, sender)) revert AccessNotAllowed(sender, sig);
}

function _msgSender() internal view override returns (address) {
address signer = msg.sender;
if (msg.data.length >= 20 && isValidForwarder[signer]) {
assembly {
signer := shr(96, calldataload(sub(calldatasize(), 20)))
}
}
return signer;
}

/**
@notice Initializes Bridge, creates and grants {_msgSender()} the admin role,
sets access control contract for bridge.
Expand Down Expand Up @@ -154,29 +106,6 @@ contract Bridge is Pausable, Context {
handler.setBurnable(tokenAddress);
}

/**
@notice Sets the nonce for the specific domainID.
@notice Only callable by address that has the right to call the specific function,
which is mapped in {functionAccess} in AccessControlSegregator contract.
@param domainID Domain ID for increasing nonce.
@param nonce The nonce value to be set.
*/
function adminSetDepositNonce(uint8 domainID, uint64 nonce) external onlyAllowed {
require(nonce > _depositCounts[domainID], "Does not allow decrements of the nonce");
_depositCounts[domainID] = nonce;
}

/**
@notice Set a forwarder to be used.
@notice Only callable by address that has the right to call the specific function,
which is mapped in {functionAccess} in AccessControlSegregator contract.
@param forwarder Forwarder address to be added.
@param valid Decision for the specific forwarder.
*/
function adminSetForwarder(address forwarder, bool valid) external onlyAllowed {
isValidForwarder[forwarder] = valid;
}

/**
@notice Changes access control contract address.
@notice Only callable by address that has the right to call the specific function,
Expand Down Expand Up @@ -210,125 +139,4 @@ contract Bridge is Pausable, Context {
IERCHandler handler = IERCHandler(handlerAddress);
handler.withdraw(data);
}

/**
@notice Initiates a transfer using a specified handler contract.
@notice Only callable when Bridge is not paused.
@param destinationDomainID ID of chain deposit will be bridged to.
@param resourceID ResourceID used to find address of handler to be used for deposit.
@param depositData Additional data to be passed to specified handler.
@param feeData Additional data to be passed to the fee handler.
@notice Emits {Deposit} event with all necessary parameters and a handler response.
@return depositNonce deposit nonce for the destination domain.
@return handlerResponse a handler response:
- ERC20Handler: responds with an empty data.
- PermissionlessGenericHandler: responds with an empty data.
*/
function deposit(
uint8 destinationDomainID,
bytes32 resourceID,
bytes calldata depositData,
bytes calldata feeData
) external payable whenNotPaused returns (uint64 depositNonce, bytes memory handlerResponse) {
if (destinationDomainID == _domainID) revert DepositToCurrentDomain();

address sender = _msgSender();
if (address(_feeHandler) == address(0)) {
require(msg.value == 0, "no FeeHandler, msg.value != 0");
} else {
// Reverts on failure
_feeHandler.collectFee{value: msg.value}(
sender,
_domainID,
destinationDomainID,
resourceID,
depositData,
feeData
);
}
address handler = _resourceIDToHandlerAddress[resourceID];
if (handler == address(0)) revert ResourceIDNotMappedToHandler();

depositNonce = ++_depositCounts[destinationDomainID];

IHandler depositHandler = IHandler(handler);
handlerResponse = depositHandler.deposit(resourceID, sender, depositData);

emit Deposit(destinationDomainID, resourceID, depositNonce, sender, depositData, handlerResponse);
return (depositNonce, handlerResponse);
}

/**
@notice Executes a deposit proposal using a specified handler contract
@notice Failed executeProposal from handler don't revert, emits {FailedHandlerExecution} event.
@param proposal Proposal which consists of:
- originDomainID ID of chain deposit originated from.
- resourceID ResourceID to be used when making deposits.
- depositNonce ID of deposit generated by origin Bridge contract.
- data Data originally provided when deposit was made.
@notice Emits {ProposalExecution} event.
@notice Behaviour: when execution fails, the handler will terminate the function with revert.
*/
function executeProposal(Proposal memory proposal) public {
Proposal[] memory proposalArray = new Proposal[](1);
proposalArray[0] = proposal;

executeProposals(proposalArray);
}

/**
@notice Executes a batch of deposit proposals using a specified handler contract for each proposal
@notice If executeProposals fails it doesn't revert, emits {FailedHandlerExecution} event.
@param proposals Array of Proposal which consists of:
- originDomainID ID of chain deposit originated from.
- resourceID ResourceID to be used when making deposits.
- depositNonce ID of deposit generated by origin Bridge contract.
- data Data originally provided when deposit was made.
@notice Emits {ProposalExecution} event for each proposal in the batch.
@notice Behaviour: when execution fails, the handler will terminate the function with revert.
*/
function executeProposals(Proposal[] memory proposals) public whenNotPaused {
if (proposals.length == 0) revert EmptyProposalsArray();

for (uint256 i = 0; i < proposals.length; i++) {
if (isProposalExecuted(proposals[i].originDomainID, proposals[i].depositNonce)) {
continue;
}

address handler = _resourceIDToHandlerAddress[proposals[i].resourceID];
bytes32 dataHash = keccak256(abi.encodePacked(handler, proposals[i].data));

IHandler depositHandler = IHandler(handler);

usedNonces[proposals[i].originDomainID][proposals[i].depositNonce / 256] |=
1 <<
(proposals[i].depositNonce % 256);

try depositHandler.executeProposal(proposals[i].resourceID, proposals[i].data) returns (
bytes memory handlerResponse
) {
emit ProposalExecution(
proposals[i].originDomainID,
proposals[i].depositNonce,
dataHash,
handlerResponse
);
} catch (bytes memory lowLevelData) {
emit FailedHandlerExecution(lowLevelData, proposals[i].originDomainID, proposals[i].depositNonce);
usedNonces[proposals[i].originDomainID][proposals[i].depositNonce / 256] &= ~(1 <<
(proposals[i].depositNonce % 256));
continue;
}
}
}

/**
@notice Returns a boolean value.
@param domainID ID of chain deposit originated from.
@param depositNonce ID of deposit generated by origin Bridge contract.
@return Boolean value depending if deposit nonce has already been used or not.
*/
function isProposalExecuted(uint8 domainID, uint256 depositNonce) public view returns (bool) {
return usedNonces[domainID][depositNonce / 256] & (1 << (depositNonce % 256)) != 0;
}
}
Loading

0 comments on commit 4ea35ff

Please sign in to comment.