improve(SpokePoolClient): Negate non-EVM SpeedUp recipients #1091
Conversation
SpeedUp events are not supported for EVM -> SVM or SVM -> any, so they can be discarded immediately.
pxrl
requested review from
nicholaspai,
fusmanii,
bmzig,
grasphoper and
dijanin-brat
| @@ -569,18 +569,23 @@ export abstract class SpokePoolClient extends BaseAbstractClient { | |||
|
|
|||
| for (const event of speedUpEvents) { | |||
| const speedUp = { ...event, originChainId: this.chainId }; | |||
| assign(this.speedUps, [speedUp.depositor, speedUp.depositId.toString()], [speedUp]); | |||
|
|
|||
| // Find deposit hash matching this speed up event and update the deposit data associated with the hash, | |||
| // if the hash+data exists. | |||
| const deposit = this.getDeposit(speedUp.depositId); | |||
There was a problem hiding this comment.
nb. This isn't robust to deterministic deposit IDs. That's wrong, but speed ups are so rarely used that it's a low-priority fix.
There was a problem hiding this comment.
what would happen if we try to getDeposit using a deterministic ID? Would it hang? We should probably skip it right?
There was a problem hiding this comment.
It should work fine to use it with a deterministic deposit ID in isolation; the challenge is if there are duplicate depositIds - in that case we may attach the speedup to the wrong one, and that would cause the eventual fill to fail.
I'll circle back to this PR and add some additional conditions to ensure we only attach a matching speedup to the deposit.
There was a problem hiding this comment.
Implemented: 07f480f
Requiring depositor to match should at least prevent incorrectly attaching the wrong speedup to a deposit.
SpeedUp events are only supported for EVM -> EVM, so any other combination can be discarded immediately.