Deploy the contracts + app together

[bpierre]

• Move the deployment part of the contracts workflow to group it with the app deployment (makes it easier to share data and execute sequentially).
• Write a “deployment context” file whenever ./deploy gets executed successfully.
• Add a script that converts this file into the env vars needed by Next.js.

Remaining:

• Only deploy the contracts on pull requests (previews are not supported by Vercel without having an account per team member)

[danielattilasimon]

I was having trouble figuring out what's happening here, and I think it's because of the .reduce() that's implementing a .map() and a .filter() rolled into one. In fact, it seems the .filter(Boolean) at the end is not needed, because the filtering's already been done (on the return value of contractNameToNextEnvVariable()). Plus, at the end of the reduction, entries is an array of arrays, and even an empty array is truthy, so every element of entries will be truthy anyway.

I think the intent can be made clearer by using separate .map() and .filter()s:

const contracts = Object.fromEntries(
  Object.entries(deploymentContext.deployedContracts)
    .map(([contractName, address]): [string | null, string] => [
      contractNameToNextEnvVariable(contractName),
      address,
    ])
    .filter((kv): kv is [string, string] => kv[0] != null)
);

Having to explicitly declare the filter function a type guard is not nice, but that's a limitation of TypeScript that should be getting improved really soon:
microsoft/TypeScript#57465

[bpierre]

Ah yes! I forgot to remove the .filter() which was precisely coming from a .map().filter(Boolean) with the typing limitation you indicated 😄. I then changed it to a .reduce(), to avoid using as on the final array (and negate all the typing checks inside of the .map()).

but you’re right, reduce()s in general are hard to read. I like the inline is solution! Much better than doing as on the final array. I was refactoring this part a little bit and I thought it could be nice to flatten things a bit by using a for?

bold/contracts/utils/deployment-artifacts-to-next-env.ts

Lines 60 to 65 in 415a356

	for (const [contractName, address] of Object.entries(deployedContracts)) {
	const envVarName = contractNameToNextEnvVariable(contractName);
	if (envVarName) {
	nextEnvVariables[envVarName] = address;
	}
	}

[danielattilasimon]

Agreed, I try to stay away from mapping objects in a functional style, because it just sucks in TS. It can be made better with the help of functional programming libraries, but I'm not sure it's worth it to pull in a dependency just for this.

I like the for loop.

[danielattilasimon]

To keep things flat when using FP, I like to use pipelines with lots of helpers:

const contracts = pipe(
  deploymentContext.deployedContracts,
  Object.entries,
  map(fst(contractNameToNextEnvVariable)),
  filter(flow(pick(0), notNull)),
  Object.fromEntries
);

Here are some of them implemented:
https://github.com/liquity/virtuoso/blob/main/tools/controller-sim/src/utils/fun.ts

This is fine for scripting but dunno how performant it is for production.

[danielattilasimon]

Of course it's ridiculous for something as simple as the above 😆

[bpierre]

Oh that’s super nice yes 🤩

[danielattilasimon]

Looks really good! 👍 👍

I am still debugging why the contract deployment is failing in CI, but I'll enable it back on once I can figure it out. In the meantime, there's no reason we can't merge this.