safe-global / safe-deployments

A collection of Safe singleton deployments
MIT License
123 stars 229 forks source link

Improve the JSON file structure to support multiple addresses per network #348

Open mmv08 opened 6 months ago

mmv08 commented 6 months ago

The big downside of the current package's structure is that consumers have to pre-load the entire JSON file with all deployments to enable deployment lookup. While I don't think there's a workaround, the JSON structure could be optimized. For example, for 1.3.0 deployments, most deployments are just mapping a chain id to one of the known addresses. Instead, it could be optimized like this:

addresses:
  eip155: 0x1
  noneip155: 0x2
  zksync: 0x3
deployments:
  1: eip155
  2: noneip155
  3: zksync
  4: eip155

Additionally, we can move the ABI outside of the main JSON file, so consumers just wanting to use the addresses don't have to load it.

EDIT 25.06.2024: After looking into optimizing the package size, I realised that the compression algorithm does a great job of compressing everything to a decent size (24kb). We could shave off a few kilobytes, but I think it doesn't have a significant impact.

So, I would suggest converting this issue into something the network and wallet teams need - supporting multiple singletons per network. I believe the proposed structure fits well still:

addresses:
  eip155: 0x1
  noneip155: 0x2
  zksync: 0x3
deployments:
  1: [eip155, noneip155]
  2: [noneip155, eip155]
  3: zksync
  4: eip155

So now the deployments will either be an array or a key from the addresses mapping. The array should be sorted by the date of deployment. The change can be done by adding a new function to avoid breaking changes.

mmv08 commented 1 month ago

While I don't think there's a workaround, the JSON structure could be optimized.

A backend service / onchain lookup solution can also be explored

katspaugh commented 1 month ago

This is a good idea. In the UI we check safe-deployments to determine supported Safes, and so does the protocol-kit. If each chain had several addresses we could be backward-compatible with both types of addresses for 1.3.0 most notably for chains like Optimism.

rmeissner commented 2 weeks ago

Couldn't we also switch around that instead of having a file per contract with list of chain ids, we could have a json per chain id with an object of addresses

mmv08 commented 2 weeks ago

Couldn't we also switch around that instead of having a file per contract with list of chain ids, we could have a json per chain id with an object of addresses

I'm curious how a bundler would handle this. Theoretically if all of the deployments are stored as static assets on the client side that may help (so you could just lookup the file needed, e.g., app.safe.global/static/deployments/chains/1.json)

nlordell commented 1 week ago

Some other considerations from our planning meeting: