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: update function and event naming for backwards compatibility #805

Open
wants to merge 25 commits into
base: svm-dev
Choose a base branch
from

Conversation

chrismaree
Copy link
Member

@chrismaree chrismaree commented Dec 18, 2024

This PR makes a single functional change: it adds back the previous fillV3Relay interface that existed prior to this audit. This method forwards to fillRelay

The rest of the PR is entirely renaming to make it an easier upgrade for integrators:

  1. The old deposit function has been removed and replaced with the colliding function name depositDeprecated_5947912356. Note: this means that anyone who fails to update their code will not be broken. However, it means that we are able to reuse the deposit name without function overloading.
  2. The bytes32 overload of depositV3 has been renamed to just deposit.
  3. Because unsafeDepositV3 has never been deployed to production, it was renamed to unsafeDeposit and only the bytes32 version was kept.
  4. The bytes32 overload of depositV3Now was renamed to depositNow.
  5. The bytes32 overload of speedUpV3Deposit was renamed to speedUpDeposit.
  6. The bytes32 version of fillV3Relay was renamed to fillRelay.
  7. The following transformation was applied to events V3FundsDeposited, RequestedSpeedUpV3Deposit, FilledV3Relay, RequestedV3SlowFill:
    1. For the bytes32 version, the V3 was dropped from the name.
    2. The legacy event with the V3 was added back to the interface, so that it persists in the ABI to make migration easier.

WIP
Signed-off-by: Chris Maree <[email protected]>
@chrismaree chrismaree changed the base branch from master to svm-dev December 18, 2024 11:05
Chris Maree and others added 2 commits December 18, 2024 12:06
WIP
Signed-off-by: Chris Maree <[email protected]>
Signed-off-by: Chris Maree <[email protected]>
@chrismaree chrismaree marked this pull request as draft December 18, 2024 14:32
Signed-off-by: Chris Maree <[email protected]>
Copy link
Member

@nicholaspai nicholaspai left a comment

Choose a reason for hiding this comment

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

I was just thinking about this, +1

uint256 inputAmount;
uint256 outputAmount;
uint256 originChainId;
uint256 depositId;
Copy link
Member Author

Choose a reason for hiding this comment

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

^

@@ -633,8 +633,8 @@ abstract contract SpokePool is
uint32 fillDeadline,
uint32 exclusivityParameter,
bytes calldata message
) public payable {
unsafeDepositV3(
) public payable override {
Copy link
Member Author

Choose a reason for hiding this comment

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

this did not have override, so I've added it to the interface.

@@ -766,8 +766,8 @@ abstract contract SpokePool is
uint32 fillDeadlineOffset,
uint32 exclusivityPeriod,
bytes calldata message
) external payable {
depositV3(
) external payable override {
Copy link
Member Author

Choose a reason for hiding this comment

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

was also missing from interface.

@@ -827,7 +827,7 @@ abstract contract SpokePool is
uint32 fillDeadlineOffset,
uint32 exclusivityPeriod,
bytes calldata message
) external payable {
) external payable override {
Copy link
Member Author

Choose a reason for hiding this comment

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

was also missing from interface.

@@ -109,22 +109,3 @@ export const sampleRateModel = {
R1: toWei(0.07).toString(),
R2: toWei(0.75).toString(),
};

export const SpokePoolFuncs = {
Copy link
Member Author

Choose a reason for hiding this comment

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

no more loverload. yay!

@@ -42,7 +42,7 @@ contract SpokePoolVerifier {
* to 0 if exclusiveRelayer is set to 0x0, and vice versa.
* @param fillDeadline Timestamp after which this deposit can no longer be filled.
*/
function deposit(
function depositV3Bytes32(
Copy link
Member Author

Choose a reason for hiding this comment

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

^

Signed-off-by: Matt Rice <[email protected]>
Signed-off-by: Matt Rice <[email protected]>
Signed-off-by: Matt Rice <[email protected]>
Signed-off-by: Matt Rice <[email protected]>
Signed-off-by: Matt Rice <[email protected]>
Signed-off-by: Matt Rice <[email protected]>
Signed-off-by: Matt Rice <[email protected]>
Signed-off-by: Matt Rice <[email protected]>
@mrice32 mrice32 marked this pull request as ready for review January 9, 2025 23:48
@mrice32 mrice32 requested review from mrice32, pxrl and bmzig as code owners January 9, 2025 23:48
@mrice32 mrice32 changed the title feat: Add legacy fill method that takes in addresses rather than bytes32 fields feat: update function and event naming for backwards compatibility Jan 9, 2025
Signed-off-by: Chris Maree <[email protected]>
Signed-off-by: Chris Maree <[email protected]>
Signed-off-by: Chris Maree <[email protected]>
Signed-off-by: Chris Maree <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants