Skip to content

Commit

Permalink
Reapply "feat(SpokePool): Introduce opt-in deterministic relay data h…
Browse files Browse the repository at this point in the history 
…ashes (#583)"

This reverts commit d363bf0.
  • Loading branch information
@mrice32
mrice32 committed Oct 9, 2024
1 parent d363bf0 commit c40c197
Show file tree
Hide file tree
Showing 4 changed files with 234 additions and 52 deletions.
210 changes: 166 additions & 44 deletions contracts/SpokePool.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ abstract contract SpokePool is

bytes32 public constant UPDATE_V3_DEPOSIT_DETAILS_HASH =
keccak256(
"UpdateDepositDetails(uint32 depositId,uint256 originChainId,uint256 updatedOutputAmount,address updatedRecipient,bytes updatedMessage)"
"UpdateDepositDetails(uint256 depositId,uint256 originChainId,uint256 updatedOutputAmount,address updatedRecipient,bytes updatedMessage)"
);

// Default chain Id used to signify that no repayment is requested, for example when executing a slow fill.
Expand Down Expand Up @@ -549,59 +549,92 @@ abstract contract SpokePool is
uint32 exclusivityPeriod,
bytes calldata message
) public payable override nonReentrant unpausedDeposits {
// Check that deposit route is enabled for the input token. There are no checks required for the output token
// which is pulled from the relayer at fill time and passed through this contract atomically to the recipient.
if (!enabledDepositRoutes[inputToken][destinationChainId]) revert DisabledRoute();

// Require that quoteTimestamp has a maximum age so that depositors pay an LP fee based on recent HubPool usage.
// It is assumed that cross-chain timestamps are normally loosely in-sync, but clock drift can occur. If the
// SpokePool time stalls or lags significantly, it is still possible to make deposits by setting quoteTimestamp
// within the configured buffer. The owner should pause deposits/fills if this is undesirable.
// This will underflow if quoteTimestamp is more than depositQuoteTimeBuffer;
// this is safe but will throw an unintuitive error.

// slither-disable-next-line timestamp
uint256 currentTime = getCurrentTime();
if (currentTime - quoteTimestamp > depositQuoteTimeBuffer) revert InvalidQuoteTimestamp();

// fillDeadline is relative to the destination chain.
// Don’t allow fillDeadline to be more than several bundles into the future.
// This limits the maximum required lookback for dataworker and relayer instances.
// Also, don't allow fillDeadline to be in the past. This poses a potential UX issue if the destination
// chain time keeping and this chain's time keeping are out of sync but is not really a practical hurdle
// unless they are significantly out of sync or the depositor is setting very short fill deadlines. This latter
// situation won't be a problem for honest users.
if (fillDeadline < currentTime || fillDeadline > currentTime + fillDeadlineBuffer) revert InvalidFillDeadline();

// If the address of the origin token is a wrappedNativeToken contract and there is a msg.value with the
// transaction then the user is sending the native token. In this case, the native token should be
// wrapped.
if (inputToken == address(wrappedNativeToken) && msg.value > 0) {
if (msg.value != inputAmount) revert MsgValueDoesNotMatchInputAmount();
wrappedNativeToken.deposit{ value: msg.value }();
// Else, it is a normal ERC20. In this case pull the token from the caller as per normal.
// Note: this includes the case where the L2 caller has WETH (already wrapped ETH) and wants to bridge them.
// In this case the msg.value will be set to 0, indicating a "normal" ERC20 bridging action.
} else {
// msg.value should be 0 if input token isn't the wrapped native token.
if (msg.value != 0) revert MsgValueDoesNotMatchInputAmount();
IERC20Upgradeable(inputToken).safeTransferFrom(msg.sender, address(this), inputAmount);
}

emit V3FundsDeposited(
_depositV3(
depositor,
recipient,
inputToken,
outputToken,
inputAmount,
outputAmount,
destinationChainId,
exclusiveRelayer,
// Increment count of deposits so that deposit ID for this spoke pool is unique.
// @dev Implicitly casts from uint32 to uint256 by padding the left-most bytes with zeros. Guarantees
// that the 24 most significant bytes are 0.
numberOfDeposits++,
quoteTimestamp,
fillDeadline,
uint32(currentTime) + exclusivityPeriod,
uint32(getCurrentTime()) + exclusivityPeriod,
message
);
}

/**
* @notice See depositV3 for details. This function is identical to depositV3 except that it does not use the
* global deposit ID counter as a deposit nonce, instead allowing the caller to pass in a deposit nonce. This
* function is designed to be used by anyone who wants to pre-compute their resultant relay data hash, which
* could be useful for filling a deposit faster and avoiding any risk of a relay hash unexpectedly changing
* due to another deposit front-running this one and incrementing the global deposit ID counter.
* @dev This is labeled "unsafe" because there is no guarantee that the depositId emitted in the resultant
* V3FundsDeposited event is unique which means that the
* corresponding fill might collide with an existing relay hash on the destination chain SpokePool,
* which would make this deposit unfillable. In this case, the depositor would subsequently receive a refund
* of `inputAmount` of `inputToken` on the origin chain after the fill deadline.
* @dev On the destination chain, the hash of the deposit data will be used to uniquely identify this deposit, so
* modifying any params in it will result in a different hash and a different deposit. The hash will comprise
* all parameters to this function along with this chain's chainId(). Relayers are only refunded for filling
* deposits with deposit hashes that map exactly to the one emitted by this contract.
* @param depositNonce The nonce that uniquely identifies this deposit. This function will combine this parameter
* with the msg.sender address to create a unique uint256 depositNonce and ensure that the msg.sender cannot
* use this function to front-run another depositor's unsafe deposit. This function guarantees that the resultant
* deposit nonce will not collide with a safe uint256 deposit nonce whose 24 most significant bytes are always 0.
* @param depositor See identically named parameter in depositV3() comments.
* @param recipient See identically named parameter in depositV3() comments.
* @param inputToken See identically named parameter in depositV3() comments.
* @param outputToken See identically named parameter in depositV3() comments.
* @param inputAmount See identically named parameter in depositV3() comments.
* @param outputAmount See identically named parameter in depositV3() comments.
* @param destinationChainId See identically named parameter in depositV3() comments.
* @param exclusiveRelayer See identically named parameter in depositV3() comments.
* @param quoteTimestamp See identically named parameter in depositV3() comments.
* @param fillDeadline See identically named parameter in depositV3() comments.
* @param exclusivityPeriod See identically named parameter in depositV3() comments.
* @param message See identically named parameter in depositV3() comments.
*/
function unsafeDepositV3(
address depositor,
address recipient,
address inputToken,
address outputToken,
uint256 inputAmount,
uint256 outputAmount,
uint256 destinationChainId,
address exclusiveRelayer,
uint256 depositNonce,
uint32 quoteTimestamp,
uint32 fillDeadline,
uint32 exclusivityPeriod,
bytes calldata message
) public payable nonReentrant unpausedDeposits {
// @dev Create the uint256 deposit ID by concatenating the msg.sender and depositor address with the inputted
// depositNonce parameter. The resultant 32 byte string will be hashed and then casted to an "unsafe"
// uint256 deposit ID. The probability that the resultant ID collides with a "safe" deposit ID is
// equal to the chance that the first 28 bytes of the hash are 0, which is too small for us to consider.

uint256 depositId = getUnsafeDepositId(msg.sender, depositor, depositNonce);
_depositV3(
depositor,
recipient,
inputToken,
outputToken,
inputAmount,
outputAmount,
destinationChainId,
exclusiveRelayer,
depositId,
quoteTimestamp,
fillDeadline,
uint32(getCurrentTime()) + exclusivityPeriod,
message
);
}
Expand Down Expand Up @@ -755,7 +788,7 @@ abstract contract SpokePool is
*/
function speedUpV3Deposit(
address depositor,
uint32 depositId,
uint256 depositId,
uint256 updatedOutputAmount,
address updatedRecipient,
bytes calldata updatedMessage,
Expand Down Expand Up @@ -1084,10 +1117,99 @@ abstract contract SpokePool is
return block.timestamp; // solhint-disable-line not-rely-on-time
}

/**
* @notice Returns the deposit ID for an unsafe deposit. This function is used to compute the deposit ID
* in unsafeDepositV3 and is provided as a convenience.
* @dev msgSenderand depositor are both used as inputs to allow passthrough depositors to create unique
* deposit hash spaces for unique depositors.
* @param msgSender The caller of the transaction used as input to produce the deposit ID.
* @param depositor The depositor address used as input to produce the deposit ID.
* @param depositNonce The nonce used as input to produce the deposit ID.
* @return The deposit ID for the unsafe deposit.
*/
function getUnsafeDepositId(
address msgSender,
address depositor,
uint256 depositNonce
) public pure returns (uint256) {
return uint256(keccak256(abi.encodePacked(msgSender, depositor, depositNonce)));
}

/**************************************
* INTERNAL FUNCTIONS *
**************************************/

function _depositV3(
address depositor,
address recipient,
address inputToken,
address outputToken,
uint256 inputAmount,
uint256 outputAmount,
uint256 destinationChainId,
address exclusiveRelayer,
uint256 depositId,
uint32 quoteTimestamp,
uint32 fillDeadline,
uint32 exclusivityDeadline,
bytes calldata message
) internal {
// Check that deposit route is enabled for the input token. There are no checks required for the output token
// which is pulled from the relayer at fill time and passed through this contract atomically to the recipient.
if (!enabledDepositRoutes[inputToken][destinationChainId]) revert DisabledRoute();

// Require that quoteTimestamp has a maximum age so that depositors pay an LP fee based on recent HubPool usage.
// It is assumed that cross-chain timestamps are normally loosely in-sync, but clock drift can occur. If the
// SpokePool time stalls or lags significantly, it is still possible to make deposits by setting quoteTimestamp
// within the configured buffer. The owner should pause deposits/fills if this is undesirable.
// This will underflow if quoteTimestamp is more than depositQuoteTimeBuffer;
// this is safe but will throw an unintuitive error.

// slither-disable-next-line timestamp
uint256 currentTime = getCurrentTime();
if (currentTime - quoteTimestamp > depositQuoteTimeBuffer) revert InvalidQuoteTimestamp();

// fillDeadline is relative to the destination chain.
// Don’t allow fillDeadline to be more than several bundles into the future.
// This limits the maximum required lookback for dataworker and relayer instances.
// Also, don't allow fillDeadline to be in the past. This poses a potential UX issue if the destination
// chain time keeping and this chain's time keeping are out of sync but is not really a practical hurdle
// unless they are significantly out of sync or the depositor is setting very short fill deadlines. This latter
// situation won't be a problem for honest users.
if (fillDeadline < currentTime || fillDeadline > currentTime + fillDeadlineBuffer) revert InvalidFillDeadline();

// If the address of the origin token is a wrappedNativeToken contract and there is a msg.value with the
// transaction then the user is sending the native token. In this case, the native token should be
// wrapped.
if (inputToken == address(wrappedNativeToken) && msg.value > 0) {
if (msg.value != inputAmount) revert MsgValueDoesNotMatchInputAmount();
wrappedNativeToken.deposit{ value: msg.value }();
// Else, it is a normal ERC20. In this case pull the token from the caller as per normal.
// Note: this includes the case where the L2 caller has WETH (already wrapped ETH) and wants to bridge them.
// In this case the msg.value will be set to 0, indicating a "normal" ERC20 bridging action.
} else {
// msg.value should be 0 if input token isn't the wrapped native token.
if (msg.value != 0) revert MsgValueDoesNotMatchInputAmount();
IERC20Upgradeable(inputToken).safeTransferFrom(msg.sender, address(this), inputAmount);
}

emit V3FundsDeposited(
inputToken,
outputToken,
inputAmount,
outputAmount,
destinationChainId,
depositId,
quoteTimestamp,
fillDeadline,
exclusivityDeadline,
depositor,
recipient,
exclusiveRelayer,
message
);
}

function _deposit(
address depositor,
address recipient,
Expand Down Expand Up @@ -1214,7 +1336,7 @@ abstract contract SpokePool is

function _verifyUpdateV3DepositMessage(
address depositor,
uint32 depositId,
uint256 depositId,
uint256 originChainId,
uint256 updatedOutputAmount,
address updatedRecipient,
Expand Down
12 changes: 6 additions & 6 deletions contracts/interfaces/V3SpokePoolInterface.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ interface V3SpokePoolInterface {
// Origin chain id.
uint256 originChainId;
// The id uniquely identifying this deposit on the origin chain.
uint32 depositId;
uint256 depositId;
// The timestamp on the destination chain after which this deposit can no longer be filled.
uint32 fillDeadline;
// The timestamp on the destination chain after which any relayer can fill the deposit.
Expand Down Expand Up @@ -102,7 +102,7 @@ interface V3SpokePoolInterface {
uint256 inputAmount,
uint256 outputAmount,
uint256 indexed destinationChainId,
uint32 indexed depositId,
uint256 indexed depositId,
uint32 quoteTimestamp,
uint32 fillDeadline,
uint32 exclusivityDeadline,
Expand All @@ -114,7 +114,7 @@ interface V3SpokePoolInterface {

event RequestedSpeedUpV3Deposit(
uint256 updatedOutputAmount,
uint32 indexed depositId,
uint256 indexed depositId,
address indexed depositor,
address updatedRecipient,
bytes updatedMessage,
Expand All @@ -128,7 +128,7 @@ interface V3SpokePoolInterface {
uint256 outputAmount,
uint256 repaymentChainId,
uint256 indexed originChainId,
uint32 indexed depositId,
uint256 indexed depositId,
uint32 fillDeadline,
uint32 exclusivityDeadline,
address exclusiveRelayer,
Expand All @@ -145,7 +145,7 @@ interface V3SpokePoolInterface {
uint256 inputAmount,
uint256 outputAmount,
uint256 indexed originChainId,
uint32 indexed depositId,
uint256 indexed depositId,
uint32 fillDeadline,
uint32 exclusivityDeadline,
address exclusiveRelayer,
Expand Down Expand Up @@ -189,7 +189,7 @@ interface V3SpokePoolInterface {

function speedUpV3Deposit(
address depositor,
uint32 depositId,
uint256 depositId,
uint256 updatedOutputAmount,
address updatedRecipient,
bytes calldata updatedMessage,
Expand Down
62 changes: 61 additions & 1 deletion test/evm/hardhat/SpokePool.Deposit.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import {
amountReceived,
MAX_UINT32,
originChainId,
zeroAddress,
} from "./constants";

const { AddressZero: ZERO_ADDRESS } = ethers.constants;
Expand Down Expand Up @@ -366,6 +365,28 @@ describe("SpokePool Depositor Logic", async function () {
_relayData.message,
];
}
function getUnsafeDepositArgsFromRelayData(
_relayData: V3RelayData,
_depositId: string,
_destinationChainId = destinationChainId,
_quoteTimestamp = quoteTimestamp
) {
return [
_relayData.depositor,
_relayData.recipient,
_relayData.inputToken,
_relayData.outputToken,
_relayData.inputAmount,
_relayData.outputAmount,
_destinationChainId,
_relayData.exclusiveRelayer,
_depositId,
_quoteTimestamp,
_relayData.fillDeadline,
_relayData.exclusivityDeadline,
_relayData.message,
];
}
beforeEach(async function () {
relayData = {
depositor: depositor.address,
Expand Down Expand Up @@ -576,6 +597,45 @@ describe("SpokePool Depositor Logic", async function () {
"ReentrancyGuard: reentrant call"
);
});
it("unsafe deposit ID", async function () {
const currentTime = (await spokePool.getCurrentTime()).toNumber();
// new deposit ID should be the uint256 equivalent of the keccak256 hash of packed {msg.sender, depositor, forcedDepositId}.
const forcedDepositId = "99";
const expectedDepositId = BigNumber.from(
ethers.utils.solidityKeccak256(
["address", "address", "uint256"],
[depositor.address, recipient.address, forcedDepositId]
)
);
expect(await spokePool.getUnsafeDepositId(depositor.address, recipient.address, forcedDepositId)).to.equal(
expectedDepositId
);
// Note: we deliberately set the depositor != msg.sender to test that the hashing algorithm correctly includes
// both addresses in the hash.
await expect(
spokePool
.connect(depositor)
.unsafeDepositV3(
...getUnsafeDepositArgsFromRelayData({ ...relayData, depositor: recipient.address }, forcedDepositId)
)
)
.to.emit(spokePool, "V3FundsDeposited")
.withArgs(
relayData.inputToken,
relayData.outputToken,
relayData.inputAmount,
relayData.outputAmount,
destinationChainId,
expectedDepositId,
quoteTimestamp,
relayData.fillDeadline,
currentTime,
recipient.address,
relayData.recipient,
relayData.exclusiveRelayer,
relayData.message
);
});
});
describe("speed up V3 deposit", function () {
const updatedOutputAmount = amountToDeposit.add(1);
Expand Down
Loading

0 comments on commit c40c197

Please sign in to comment.