hyperlane-monorepo icon indicating copy to clipboard operation
hyperlane-monorepo copied to clipboard

Scraper: addresses in DB should always be 32 bytes

Open tkporter opened this issue 1 year ago • 1 comments

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:

  • Syncing a new scraper DB is needed. This is because messages that have e.g. been sent from Arbitrum to Neutron have had their recipient addresses truncated to 20 bytes. Probably best to wait to actually roll this out till much later, see https://github.com/hyperlane-xyz/hyperlane-monorepo/issues/3356 for some thoughts there
  • the explorer UI makes GraphQL queries and assumes these addresses are 20 bytes throughout. The postgresByteaToString function is used and the length isn't checked for addresses. Therefore we'll need to update the explorer UI logic to correctly parse 32 byte addresses from the DB into protocol-specific addresses. This is tracked in https://github.com/hyperlane-xyz/hyperlane-explorer/issues/61

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 avatar Feb 29 '24 14:02 tkporter

~~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

tkporter avatar Feb 29 '24 16:02 tkporter