feat: add updated UMIP invalidity cases #831
Conversation
Signed-off-by: bennett <[email protected]>
Signed-off-by: bennett <[email protected]>
Co-authored-by: Dong-Ha Kim <[email protected]>
This reverts commit 7dcd6b8.
Signed-off-by: bennett <[email protected]>
Signed-off-by: bennett <[email protected]>
Signed-off-by: bennett <[email protected]>
Signed-off-by: bennett <[email protected]>
Signed-off-by: bennett <[email protected]>
Signed-off-by: bennett <[email protected]>
Signed-off-by: bennett <[email protected]>
Signed-off-by: bennett <[email protected]>
Signed-off-by: bennett <[email protected]>
Signed-off-by: bennett <[email protected]>
Signed-off-by: bennett <[email protected]>
Signed-off-by: bennett <[email protected]>
| @@ -805,8 +808,15 @@ export class BundleDataClient { | |||
| return; | |||
| } | |||
|
|
|||
| // A deposit refund for a deposit is invalid if the depositor has a bytes32 address input for an EVM chain. It is valid otherwise. | |||
There was a problem hiding this comment.
Isn't it more inclusive to check if the address is a bytes20 address? Can use an ethers is address function for example.
I can imagine that an input here is not bytes32 but also not a valid address.
There was a problem hiding this comment.
Fair enough. Updated here: 59315b8. Although, note that we assume that the refund will be valid all times if the chain is non-evm, which is probably fine for SVM, since the event should always give us a bytes32.
| @@ -914,11 +924,32 @@ export class BundleDataClient { | |||
| bundleInvalidFillsV3.push(fill); | |||
| return; | |||
| } | |||
| // If the fill's repayment address is not a valid EVM address and the repayment chain is an EVM chain, the fill is invalid. | |||
| if (chainIsEvm(fill.repaymentChainId) && isAddressBytes32(fill.relayer)) { | |||
There was a problem hiding this comment.
I would do this checking higher up when creating bundle refunds https://github.com/across-protocol/sdk/blob/bz/newInvalidityCases/src/clients/BundleDataClient/BundleDataClient.ts#L76
Here is also where we check if the fill is a slow fill in which case we ignore it
There was a problem hiding this comment.
If I move this to that function linked, then it would need to be async, right? Since we need to fallback to the msg.sender if this first conditional is true. Also, moving it up would mean we can't reliably update bundleInvalidFillsV3, right? Unless that function tells us if it successfully updated the dictionary or not.
There was a problem hiding this comment.
Never mind. Updated here: 76de69a
| @@ -816,7 +826,7 @@ export class BundleDataClient { | |||
| // that would eliminate any deposits in this bundle with a very low fillDeadline like equal to 0 | |||
| // for example. Those should be impossible to create but technically should be included in this | |||
| // bundle of refunded deposits. | |||
| if (deposit.fillDeadline < bundleBlockTimestamps[destinationChainId][1]) { | |||
| if (deposit.fillDeadline < bundleBlockTimestamps[destinationChainId][1] && refundIsValid(deposit)) { | |||
There was a problem hiding this comment.
I'm a bit uncomfortable adding this check here in case it's not all inclusive of all expired deposits. Can we instead add it to this function such that we are safer about all expired deposits refund cases go through this check? https://github.com/across-protocol/sdk/blob/bz/newInvalidityCases/src/clients/BundleDataClient/BundleDataClient.ts#L60
There was a problem hiding this comment.
That makes sense. Refactored here: 59315b8
Signed-off-by: bennett <[email protected]>
Signed-off-by: bennett <[email protected]>
Signed-off-by: bennett <[email protected]>
Signed-off-by: bennett <[email protected]>
A deposit refund is now "invalid" if the depositor specified cannot be repaid (since the origin chain is an EVM chain and the depositor address is not a bytes20 address).
A fill is invalid if the relayer supplies a 32 byte address and requests repayment on an EVM chain and the
msg.senderis a 32-byte address when inspecting the destination chain fill.