Skip to content

feat(spoke-pool): Add speedUpDeposit method #56

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Feb 28, 2022
Merged

feat(spoke-pool): Add speedUpDeposit method #56

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
42ff762
feat(spoke-pool): Add speedUpDeposit method
nicholaspai Feb 27, 2022
4d6ed7b
add tests
nicholaspai Feb 28, 2022
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
90 changes: 68 additions & 22 deletions contracts/SpokePool.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,12 @@ abstract contract SpokePool is SpokePoolInterface, Testable, Lockable, MultiCall
address recipient,
address indexed depositor
);
event RequestedSpeedUpDeposit(
uint64 newRelayerFeePct,
uint32 indexed depositId,
address indexed depositor,
bytes depositorSignature
);
event FilledRelay(
bytes32 indexed relayHash,
uint256 totalRelayAmount,
Expand Down Expand Up @@ -252,6 +258,37 @@ abstract contract SpokePool is SpokePoolInterface, Testable, Lockable, MultiCall
numberOfDeposits += 1;
}

// Convenience method that depositor can use to signal to relayer to use updated fee.
/**
* @notice Convenience method that depositor can use to signal to relayer to use updated fee.
* @dev Relayer should only use events emitted by this function to submit fills with updated fees, otherwise they
* risk their fills getting disputed for being invalid, for example if the depositor never actually signed the
* update fee message.
* @dev This function will revert if the depositor did not sign a message containing the updated fee for the deposit
* ID stored in this contract. If the deposit ID is for another contract, or the depositor address is incorrect,
* or the updated fee is incorrect, then the signature will not match and this function will revert.
* @param depositor Signer of the update fee message who originally submitted the deposit. If the deposit doesn't
* exist, then the relayer will not be able to fill any relay, so the caller should validate that the depositor
* did in fact submit a relay.
* @param newRelayerFeePct New relayer fee that relayers can use.
* @param depositId Deposit to update fee for that originated in this contract.
* @param depositorSignature Signed message containing the depositor address, this contract chain ID, the updated
* relayer fee %, and the deposit ID. This signature is produced by signing a hash of data according to the
* EIP-191 standard. See more in the `_verifyUpdateRelayerFeeMessage()` comments.
*/
function speedUpDeposit(
Copy link
Member

@chrismaree chrismaree Feb 27, 2022

Choose a reason for hiding this comment

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

nice. test please

address depositor,
uint64 newRelayerFeePct,
uint32 depositId,
bytes memory depositorSignature
) public nonReentrant {
_verifyUpdateRelayerFeeMessage(depositor, chainId(), newRelayerFeePct, depositId, depositorSignature);

// Assuming the above checks passed, a relayer can take the signature and the updated relayer fee information
// from the following event to submit a fill with an updated fee %.
emit RequestedSpeedUpDeposit(newRelayerFeePct, depositId, depositor, depositorSignature);
}

/**************************************
* RELAYER FUNCTIONS *
**************************************/
Expand Down Expand Up @@ -302,26 +339,7 @@ abstract contract SpokePool is SpokePoolInterface, Testable, Lockable, MultiCall
uint32 depositId,
bytes memory depositorSignature
) public nonReentrant {
// Grouping the signature validation logic into brackets to address stack too deep error.
{
// Depositor should have signed a hash of the relayer fee % to update to and information uniquely identifying
// the deposit to relay. This ensures that this signature cannot be re-used for other deposits. The version
// string is included as a precaution in case this contract is upgraded.
// Note: we use encode instead of encodePacked because it is more secure, more in the "warning" section
// here: https://docs.soliditylang.org/en/v0.8.11/abi-spec.html#non-standard-packed-mode
bytes32 expectedDepositorMessageHash = keccak256(
abi.encode("ACROSS-V2-FEE-1.0", newRelayerFeePct, depositId, originChainId)
);

// Check the hash corresponding to the https://eth.wiki/json-rpc/API#eth_sign[`eth_sign`]
// JSON-RPC method as part of EIP-191. We use OZ's signature checker library with adds support for
// EIP-1271 which can verify messages signed by smart contract wallets like Argent and Gnosis safes.
// If the depositor signed a message with a different updated fee (or any other param included in the
// above keccak156 hash), then this will revert.
bytes32 ethSignedMessageHash = ECDSA.toEthSignedMessageHash(expectedDepositorMessageHash);

_verifyDepositorUpdateFeeMessage(depositor, ethSignedMessageHash, depositorSignature);
}
_verifyUpdateRelayerFeeMessage(depositor, originChainId, newRelayerFeePct, depositId, depositorSignature);

// Now follow the default `fillRelay` flow with the updated fee and the original relay hash.
RelayData memory relayData = RelayData({
Expand Down Expand Up @@ -496,8 +514,36 @@ abstract contract SpokePool is SpokePoolInterface, Testable, Lockable, MultiCall

function _bridgeTokensToHubPool(SpokePoolInterface.RelayerRefundLeaf memory relayerRefundLeaf) internal virtual;

// Allow L2 to implement chain specific recovering of signers from signatures because some L2s might not support
// ecrecover, such as those with account abstraction like ZKSync.
function _verifyUpdateRelayerFeeMessage(
address depositor,
uint256 originChainId,
uint64 newRelayerFeePct,
uint32 depositId,
bytes memory depositorSignature
) internal {
// A depositor can request to speed up an un-relayed deposit by signing a hash containing the relayer
// fee % to update to and information uniquely identifying the deposit to relay. This information ensures
// that this signature cannot be re-used for other deposits. The version string is included as a precaution
// in case this contract is upgraded.
// Note: we use encode instead of encodePacked because it is more secure, more in the "warning" section
// here: https://docs.soliditylang.org/en/v0.8.11/abi-spec.html#non-standard-packed-mode
bytes32 expectedDepositorMessageHash = keccak256(
abi.encode("ACROSS-V2-FEE-1.0", newRelayerFeePct, depositId, originChainId)
);

// Check the hash corresponding to the https://eth.wiki/json-rpc/API#eth_sign[`eth_sign`]
// JSON-RPC method as part of EIP-191. We use OZ's signature checker library which adds support for
// EIP-1271 which can verify messages signed by smart contract wallets like Argent and Gnosis safes.
// If the depositor signed a message with a different updated fee (or any other param included in the
// above keccak156 hash), then this will revert.
bytes32 ethSignedMessageHash = ECDSA.toEthSignedMessageHash(expectedDepositorMessageHash);

_verifyDepositorUpdateFeeMessage(depositor, ethSignedMessageHash, depositorSignature);
}

// This function is isolated and made virtual to allow different L2's to implement chain specific recovery of
// signers from signatures because some L2s might not support `ecrecover`, such as those with account abstraction
// like ZKSync.
function _verifyDepositorUpdateFeeMessage(
address depositor,
bytes32 ethSignedMessageHash,
Expand Down
51 changes: 51 additions & 0 deletions test/SpokePool.Relay.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,57 @@ describe("SpokePool Relayer Logic", async function () {
)
).to.be.revertedWith("relay filled");
});
it("Can signal to relayer to use updated fee", async function () {
const spokePoolChainId = await spokePool.chainId();
const { signature } = await modifyRelayHelper(
consts.modifiedRelayerFeePct,
consts.firstDepositId.toString(),
spokePoolChainId.toString(),
depositor
);
await expect(
spokePool
.connect(relayer)
.speedUpDeposit(depositor.address, consts.modifiedRelayerFeePct, consts.firstDepositId, signature)
)
.to.emit(spokePool, "RequestedSpeedUpDeposit")
.withArgs(consts.modifiedRelayerFeePct, consts.firstDepositId, depositor.address, signature);

// Reverts if any param passed to function is changed.
await expect(
spokePool
.connect(relayer)
.speedUpDeposit(relayer.address, consts.modifiedRelayerFeePct, consts.firstDepositId, signature)
).to.be.reverted;
await expect(spokePool.connect(relayer).speedUpDeposit(depositor.address, "0", consts.firstDepositId, signature)).to
.be.reverted;
await expect(
spokePool
.connect(relayer)
.speedUpDeposit(depositor.address, consts.modifiedRelayerFeePct, consts.firstDepositId + 1, signature)
).to.be.reverted;
await expect(
spokePool
.connect(relayer)
.speedUpDeposit(depositor.address, consts.modifiedRelayerFeePct, consts.firstDepositId, "0xrandombytes")
).to.be.reverted;
const { signature: incorrectOriginChainIdSignature } = await modifyRelayHelper(
consts.modifiedRelayerFeePct,
consts.firstDepositId.toString(),
consts.originChainId.toString(),
depositor
);
await expect(
spokePool
.connect(relayer)
.speedUpDeposit(
depositor.address,
consts.modifiedRelayerFeePct,
consts.firstDepositId,
incorrectOriginChainIdSignature
)
).to.be.reverted;
});
it("Can fill relay with updated fee by including proof of depositor's agreement", async function () {
// The relay should succeed just like before with the same amount of tokens pulled from the relayer's wallet,
// however the filled amount should have increased since the proportion of the relay filled would increase with a
Expand Down