Remove precompiles from the Lockup repo

[smol-ninja]

Problem

Since the bytecode of Lockup now contains placeholders for library addresses, it has become complicated to generate the correct bytecode and use it to make deployments.

So far, we have been using a "jugaad" to make the tests pass which is that we've added dummy libraries addresses (which I intent to remove in #1158) and use them in precompile bytecode.

However, this mean that on a new chain, one would have to deploy the libraries first and then replace placeholders with correct library addresses, before making the deployment. This also creates issues for us in Precompiles_Test. All of this has made it difficult to maintain and generate precompiles using the shell script.

It is much easier to use custom deployment instructions or to use Foundry to make the deployment directly.

Therefore, I recommend to remove precompiles entirely from the Lockup repo.


Recommendations

1. Remove update-precompiles.sh
2. Remove precompiles directory
3. Remove Precompiles.t.sol

RFC @sablier-labs/solidity.

[andreivladbrg]

There indeed is a problem, which I also faced when I did the test deployments for the contracts and wanted to point it out in this slack thread.

Your recommendations make sense given the situation.

Another solution to this would be to declare two mappings with all libraries addresses for all chains, and then change the bytecode in the constructor based on the chain id that contracts are being used.

mapping(uint256 chainId => address vestingMath) internal vestingMaths;
mapping(uint256 chainId => address helper) internal helpers;

This solution would require considerable more work on our side, and the maintance cost is not negligible, thus I believe is not worth it.

Regarding why we have the precompiles in the first place, the rationale was this:

https://github.com/sablier-labs/v2-core/blob/0577d2a4b7482a9e788ea698be3993d13d8b910a/precompiles/Precompiles.sol#L11-L12

which IMO is not a good motivation because if one either uses the bytecode deployed or not, the functionalities of the contract would be the same. the only two different things between the optimized bytecode and the one produced without via_ir is (i) its size (ii) gas cost of the function called. and i believe none of these 2 things are relevant to one integrating our protocol since (i) we are deploying the contracts to all chains and (ii) we are providing a benchmark for the live version

also, even us, where we could use them we are not. we are re-deploying the contracts

My conclusion is that given what they are meant to be used for and given what requires us to maintain them, it makes no sense to keep them, so we can completely remove the precompiles from our codebase. Let's focus on user requests and what it is actually used (ref to the broker removal - they are not used so we decided to remove them)

@PaulRBerg IIRC you were having a strong opinion about them, is it still the case?

[smol-ninja]

I have made some changes with which we can keep precompiles (see #1158).

• update precompiles shell script
• adjustActualBytecode

But this is just a way to fix the test, until we make a decision. I don't think we should keep it though.

Another solution to this would be to declare two mappings with all libraries addresses for all chains, and then change the bytecode in the constructor based on the chain id that contracts are being used.

Yeah something similar I did but just hardcoded. In any approach, we will still have to maintain the addresses.

[PaulRBerg]

Happy to jettison the precompiles. I don't remember any developer/integrator asking any question about them.