Skip to content

Commit

Permalink
feat: refactor handlers so they return deposit data intead of handler…
Browse files Browse the repository at this point in the history
… response (#15)

* Return deposit data from the handlers

* FIx ERC20 handler data deconstruction

* Update tests

* Update solidity docs

* Lint
  • Loading branch information
mpetrun5 authored Jan 15, 2024
1 parent 4ea35ff commit d402891
Show file tree
Hide file tree
Showing 14 changed files with 56 additions and 113 deletions.
17 changes: 7 additions & 10 deletions src/contracts/Router.sol
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,7 @@ contract Router is Context {
bytes32 resourceID,
uint64 depositNonce,
address indexed user,
bytes data,
bytes handlerResponse
bytes data
);

modifier onlyAllowed() {
Expand Down Expand Up @@ -80,18 +79,15 @@ contract Router is Context {
@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.
@notice Emits {Deposit} event with all necessary parameters.
@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 whenBridgeNotPaused returns (uint64 depositNonce, bytes memory handlerResponse) {
) external payable whenBridgeNotPaused returns (uint64 depositNonce) {
if (destinationDomainID == _domainID) revert DepositToCurrentDomain();

address sender = _msgSender();
Expand All @@ -114,10 +110,11 @@ contract Router is Context {

depositNonce = ++_depositCounts[destinationDomainID];


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

emit Deposit(destinationDomainID, resourceID, depositNonce, sender, depositData, handlerResponse);
return (depositNonce, handlerResponse);
emit Deposit(destinationDomainID, resourceID, depositNonce, sender, handlerDepositData);
return depositNonce;
}
}
11 changes: 9 additions & 2 deletions src/contracts/handlers/ERC20Handler.sol
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,10 @@ contract ERC20Handler is IHandler, ERCHandlerHelpers, ERC20Safe {
bytes calldata data
) external override onlyBridge returns (bytes memory) {
uint256 amount;
(amount) = abi.decode(data, (uint));
uint256 destinationRecipientAddressLen;
bytes memory destinationRecipientAddress;
(amount, destinationRecipientAddressLen) = abi.decode(data, (uint, uint));
destinationRecipientAddress = bytes(data[64:64 + destinationRecipientAddressLen]);

address tokenAddress = _resourceIDToTokenContractAddress[resourceID];
if (!_tokenContractAddressToTokenProperties[tokenAddress].isWhitelisted)
Expand All @@ -57,7 +60,11 @@ contract ERC20Handler is IHandler, ERCHandlerHelpers, ERC20Safe {
lockERC20(tokenAddress, depositor, address(this), amount);
}

return abi.encodePacked(convertToInternalBalance(tokenAddress, amount));
return abi.encodePacked(
convertToInternalBalance(tokenAddress, amount),
destinationRecipientAddressLen,
destinationRecipientAddress
);
}

/**
Expand Down
6 changes: 3 additions & 3 deletions src/contracts/handlers/ERCHandlerHelpers.sol
Original file line number Diff line number Diff line change
Expand Up @@ -121,17 +121,17 @@ contract ERCHandlerHelpers is IERCHandler {
@param tokenAddress Address of contract to be used when executing proposals.
@param amount Decimals value to be set for {contractAddress}.
*/
function convertToInternalBalance(address tokenAddress, uint256 amount) internal view returns (bytes memory) {
function convertToInternalBalance(address tokenAddress, uint256 amount) internal view returns (uint256) {
Decimals memory decimals = _tokenContractAddressToTokenProperties[tokenAddress].decimals;
uint256 convertedBalance;
if (!decimals.isSet) {
return "";
return amount;
} else if (decimals.externalDecimals >= DEFAULT_DECIMALS) {
convertedBalance = amount / (10 ** (decimals.externalDecimals - DEFAULT_DECIMALS));
} else {
convertedBalance = amount * (10 ** (DEFAULT_DECIMALS - decimals.externalDecimals));
}

return abi.encodePacked(convertedBalance);
return convertedBalance;
}
}
3 changes: 2 additions & 1 deletion src/contracts/handlers/PermissionlessGenericHandler.sol
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ contract PermissionlessGenericHandler is IHandler {
After this, the target contract will get the following:
executeFuncSignature(address executionDataDepositor, uint[] uintArray, address addr)
*/
function deposit(bytes32 resourceID, address depositor, bytes calldata data) external view returns (bytes memory) {
function deposit(bytes32 resourceID, address depositor, bytes calldata data) external pure returns (bytes memory) {
require(data.length >= 76, "Incorrect data length"); // 32 + 2 + 1 + 1 + 20 + 20

uint256 maxFee;
Expand Down Expand Up @@ -141,6 +141,7 @@ contract PermissionlessGenericHandler is IHandler {

require(maxFee < MAX_FEE, "requested fee too large");
require(depositor == executionDataDepositor, "incorrect depositor in deposit data");
return data;
}

/**
Expand Down
2 changes: 0 additions & 2 deletions test/contractBridge/depositERC20.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,6 @@ describe("Bridge - [deposit - ERC20]", () => {
expectedDepositNonce,
await depositorAccount.getAddress(),
depositData.toLowerCase(),
"0x",
);

const depositTx2 = routerInstance
Expand All @@ -203,7 +202,6 @@ describe("Bridge - [deposit - ERC20]", () => {
expectedDepositNonce + 1,
await depositorAccount.getAddress(),
depositData.toLowerCase(),
"0x",
);
});

Expand Down
35 changes: 10 additions & 25 deletions test/e2e/erc20/decimals/bothChainsNot18Decimals.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import {
deployBridgeContracts,
createResourceID,
createERCDepositData,
createDepositProposalDataFromHandlerResponse,
toHex,
} from "../../../helpers";
import type {
Expand Down Expand Up @@ -206,6 +205,9 @@ describe("E2E ERC20 - Two EVM Chains both with decimal places != 18", () => {

await expect(originDepositTx).not.to.be.reverted;

const originExpectedDepositData =
toHex(relayerConvertedAmount.toString(), 32) +
originDepositData.substring(66);
// check that deposited amount converted to 18 decimal places is
// emitted in handlerResponse
await expect(originDepositTx)
Expand All @@ -215,23 +217,13 @@ describe("E2E ERC20 - Two EVM Chains both with decimal places != 18", () => {
originResourceID.toLowerCase(),
expectedDepositNonce,
await depositorAccount.getAddress(),
originDepositData.toLowerCase(),
toHex(relayerConvertedAmount.toString(), 32),
);

// this mocks depositProposal data for executing on
// destination chain which is returned from relayers
const originDepositProposalData =
await createDepositProposalDataFromHandlerResponse(
originDepositTx,
20,
await recipientAccount.getAddress(),
originExpectedDepositData.toLowerCase(),
);

const originDomainProposal = {
originDomainID: originDomainID,
depositNonce: expectedDepositNonce,
data: originDepositProposalData,
data: originExpectedDepositData,
resourceID: destinationResourceID,
};

Expand Down Expand Up @@ -282,6 +274,9 @@ describe("E2E ERC20 - Two EVM Chains both with decimal places != 18", () => {
);
await expect(destinationDepositTx).not.to.be.reverted;

const destinationExepectedDepositData =
toHex(relayerConvertedAmount.toString(), 32) +
destinationDepositData.substring(66);
// check that deposited amount converted to 18 decimal places is
// emitted in handlerResponse
await expect(destinationDepositTx)
Expand All @@ -291,23 +286,13 @@ describe("E2E ERC20 - Two EVM Chains both with decimal places != 18", () => {
destinationResourceID.toLowerCase(),
expectedDepositNonce,
await recipientAccount.getAddress(),
destinationDepositData.toLowerCase(),
toHex(relayerConvertedAmount.toString(), 32),
);

// this mocks depositProposal data for executing on
// destination chain which is returned from relayers
const destinationDepositProposalData =
await createDepositProposalDataFromHandlerResponse(
destinationDepositTx,
20,
await depositorAccount.getAddress(),
destinationExepectedDepositData.toLowerCase(),
);

const destinationDomainProposal = {
originDomainID: destinationDomainID,
depositNonce: expectedDepositNonce,
data: destinationDepositProposalData,
data: destinationExepectedDepositData,
resourceID: originResourceID,
};

Expand Down
14 changes: 2 additions & 12 deletions test/e2e/erc20/decimals/oneChainNot18Decimals.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import {
deployBridgeContracts,
createResourceID,
createERCDepositData,
createDepositProposalDataFromHandlerResponse,
getDepositEventData,
} from "../../../helpers";
import type {
Bridge,
Expand Down Expand Up @@ -224,19 +224,10 @@ describe("E2E ERC20 - Two EVM Chains, one with decimal places == 18, other with
);
await expect(originDepositTx).not.to.be.reverted;

// this mocks depositProposal data for executing on
// destination chain which is returned from relayers
const originDepositProposalData =
await createDepositProposalDataFromHandlerResponse(
originDepositTx,
20,
await recipientAccount.getAddress(),
);

const originDomainProposal = {
originDomainID: originDomainID,
depositNonce: expectedDepositNonce,
data: originDepositProposalData,
data: await getDepositEventData(originDepositTx),
resourceID: destinationResourceID,
};

Expand Down Expand Up @@ -297,7 +288,6 @@ describe("E2E ERC20 - Two EVM Chains, one with decimal places == 18, other with
expectedDepositNonce,
await recipientAccount.getAddress(),
destinationDepositData.toLowerCase(),
"0x",
);

// Recipient should have a balance of 0 (deposit amount)
Expand Down
14 changes: 2 additions & 12 deletions test/e2e/erc20/decimals/oneChainWith0Decimals.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import {
deployBridgeContracts,
createResourceID,
createERCDepositData,
createDepositProposalDataFromHandlerResponse,
getDepositEventData,
} from "../../../helpers";
import type {
Bridge,
Expand Down Expand Up @@ -226,19 +226,10 @@ describe("E2E ERC20 - Two EVM Chains, one with decimal places == 18, other with

await expect(originDepositTx).not.to.be.reverted;

// this mocks depositProposal data for executing on
// destination chain which is returned from relayers
const originDepositProposalData =
await createDepositProposalDataFromHandlerResponse(
originDepositTx,
20,
await recipientAccount.getAddress(),
);

const originDomainProposal = {
originDomainID: originDomainID,
depositNonce: expectedDepositNonce,
data: originDepositProposalData,
data: getDepositEventData(originDepositTx),
resourceID: destinationResourceID,
};

Expand Down Expand Up @@ -299,7 +290,6 @@ describe("E2E ERC20 - Two EVM Chains, one with decimal places == 18, other with
expectedDepositNonce,
await recipientAccount.getAddress(),
destinationDepositData.toLowerCase(),
"0x",
);

// Recipient should have a balance of 0 (deposit amount)
Expand Down
42 changes: 13 additions & 29 deletions test/e2e/erc20/decimals/roundingLoss.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import {
createResourceID,
createERCDepositData,
toHex,
createDepositProposalDataFromHandlerResponse,
getDepositEventData,
} from "../../../helpers";
import type {
Bridge,
Expand Down Expand Up @@ -216,32 +216,24 @@ describe("E2E ERC20 - Two EVM Chains both with decimal places != 18 with roundin
);
await expect(originDepositTx).not.to.be.reverted;

// check that deposited amount converted to 18 decimal places is
// emitted in handlerResponse
// check that deposited amount converted to 18 decimal places is emitted in data
const originExpectedDepositAmount =
toHex(originRelayerConvertedAmount.toString(), 32) +
originDepositData.substring(66);
await expect(originDepositTx)
.to.emit(originRouterInstance, "Deposit")
.withArgs(
destinationDomainID,
originResourceID.toLowerCase(),
expectedDepositNonce,
await depositorAccount.getAddress(),
originDepositData.toLowerCase(),
toHex(originRelayerConvertedAmount.toString(), 32),
);

// this mocks depositProposal data for executing on
// destination chain which is returned from relayers
const originDepositProposalData =
await createDepositProposalDataFromHandlerResponse(
originDepositTx,
20,
await recipientAccount.getAddress(),
originExpectedDepositAmount.toLowerCase(),
);

const originDomainProposal = {
originDomainID: originDomainID,
depositNonce: expectedDepositNonce,
data: originDepositProposalData,
data: getDepositEventData(originDepositTx),
resourceID: destinationResourceID,
};

Expand Down Expand Up @@ -292,32 +284,24 @@ describe("E2E ERC20 - Two EVM Chains both with decimal places != 18 with roundin
);
await expect(destinationDepositTx).not.to.be.reverted;

// check that deposited amount converted to 18 decimal places is
// emitted in handlerResponse
// check that deposited amount converted to 18 decimal places is emitted in data
const destinationExpectedDepositData =
toHex(destinationRelayerConvertedAmount.toString(), 32) +
destinationDepositData.substring(66);
await expect(destinationDepositTx)
.to.emit(destinationRouterInstance, "Deposit")
.withArgs(
originDomainID,
destinationResourceID,
expectedDepositNonce,
await recipientAccount.getAddress(),
destinationDepositData.toLowerCase(),
toHex(destinationRelayerConvertedAmount.toString(), 32),
);

// this mocks depositProposal data for executing on
// destination chain which is returned from relayers
const destinationDepositProposalData =
await createDepositProposalDataFromHandlerResponse(
destinationDepositTx,
20,
await depositorAccount.getAddress(),
destinationExpectedDepositData.toLowerCase(),
);

const destinationDomainProposal = {
originDomainID: destinationDomainID,
depositNonce: expectedDepositNonce,
data: destinationDepositProposalData,
data: await getDepositEventData(destinationDepositTx),
resourceID: originResourceID,
};

Expand Down
2 changes: 0 additions & 2 deletions test/handlers/erc20/deposit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,6 @@ describe("ERC20Handler - [Deposit ERC20]", () => {
lenRecipientAddress,
recipientAccount,
).toLowerCase(),
"0x",
);
});

Expand Down Expand Up @@ -151,7 +150,6 @@ describe("ERC20Handler - [Deposit ERC20]", () => {
lenRecipientAddress,
recipientAccount,
),
"0x",
);
});

Expand Down
2 changes: 0 additions & 2 deletions test/handlers/fee/basic/collectFee.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,6 @@ describe("BasicFeeHandler - [collectFee]", () => {
expectedDepositNonce,
await depositorAccount.getAddress(),
erc20depositData.toLowerCase(),
"0x",
);

await expect(depositTx)
Expand Down Expand Up @@ -415,7 +414,6 @@ describe("BasicFeeHandler - [collectFee]", () => {
expectedDepositNonce,
await depositorAccount.getAddress(),
erc20depositData.toLowerCase(),
"0x",
);

await expect(depositTx)
Expand Down
Loading

0 comments on commit d402891

Please sign in to comment.