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(SpokePoolClient): Append messageHash to deposit events #846

Open
wants to merge 4 commits into
base: pxrl/slowFillRequest
Choose a base branch
from

Conversation

pxrl
Copy link
Contributor

@pxrl pxrl commented Jan 30, 2025

In anticipation of needing to use the deposit messageHash when matching fills to deposits, precompute the messageHash when deposit events are ingested.

@pxrl pxrl requested review from bmzig and nicholaspai January 30, 2025 15:10
@@ -54,6 +56,7 @@ const SortableEventSS = {
};

const V3DepositSS = {
messageHash: defaulted(string(), UNDEFINED_MESSAGE_HASH),
Copy link
Contributor Author

@pxrl pxrl Jan 30, 2025

Choose a reason for hiding this comment

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

defaulted() will establish an instance of messageHash with the value <UNDEFINED_MESSAGE_HASH> (""). I looked at superstruct for ways of dynamically assigning this (i.e. computing on the fly based on the rest of the object). It kinda seems like superstruct can support this, but we have a lot of nested structures in our object validations and I lost patience with it.

In any case, this is backwards compatible because we can populate messageHash values for existing deposits that were not posted with the field. I'd also consider dropping it from data posted to arweave, since there is some odd redundancy between message and messageHash, but we don't expect to verify one from the other when we pull them down simultaneously.

Comment on lines -356 to +362
-1,
toBN(-1),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It seems like this was probably left out of the recent BigNumber depositId migration.

@pxrl pxrl changed the base branch from master to pxrl/slowFillRequest January 30, 2025 15:23
pxrl added 4 commits January 31, 2025 14:24
In anticipation of needing to use the deposit messageHash when matching
fills to deposits, precompute the messageHash when deposit events are
ingested.
`getRelayDataHash()` can no longer be used for hashing a SlowFillRequest
because the new event types emit `messageHash` instead of `message`.
This is OK in the SpokePoolClient because it only needs a very simple
method of indexing SlowFillRequest events.
@pxrl pxrl force-pushed the pxrl/depositMessageHash branch from 1476a3c to 696ec18 Compare January 31, 2025 14:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant