hyperlane-xyz / hyperlane-monorepo

The home for Hyperlane core contracts, sdk packages, and other infrastructure
https://hyperlane.xyz
Other
333 stars 359 forks source link

Scraper: addresses in DB should always be 32 bytes #3330

Closed tkporter closed 3 weeks ago

tkporter commented 8 months ago

Problem

We use the bytea type in the DB schema for anything "bytes" -- this includes long arbitrary length things like message bodies and things that we may consider "fixed length", like message senders, recipients, or mailbox contracts. The common practice in PostgreSQL is to use bytea even if the data is fixed length, so we should keep using bytea.

However at the moment the scraper uses address_to_bytes (to go from H256 -> 20 byte Vec) and bytes_to_address (20 byte Vec -> H256) extensively. Things like the message sender, recipient, origin and destination mailboxes etc use these functions to be stored in the db as a 20 byte representation.

This isn't compatible with non-EVM chains, where these addresses are up to 32 bytes in length.

Solution

Because our protocol frequently assumes 32 byte lengths of these things (e.g. senders / recipients are serialized in the message as 32 bytes each, origin mailboxes are signed as 32 bytes, etc), we should change the scraper logic to write these things to the db as 32 bytes.

This has implications on rollout:

So imo we shouldn't worry about rolling this out - we'll have a number of backward incompatible changes as we move toward supporting Solana + Cosmos, and we can roll them out altogether in a new scraper instance later on. This is tracked in https://github.com/hyperlane-xyz/hyperlane-monorepo/issues/3356 and https://github.com/hyperlane-xyz/hyperlane-monorepo/issues/4274

tkporter commented 8 months ago

3 days because of syncing a new scraper DB. I'm not certain we have any practices around doing it in a manner that avoids downtime, so we'll have to figure that out. By this I mean we'll probably wanna keep the existing scraper running while we spin up a new scraper that works against a new DB

edit: I no longer think we need to roll anything out here, and think this should wait till later, which cuts this down by a day imo