Understanding the use of precompiled contracts in tests

[PaulRBerg]

This is what it is said in the README in the Foundry Tests section:

By default, the differential test suite deploys precompiled versions of both the optimized and reference contracts.

However, by peeking at the Foundry tests, I can see that you are not not deploying pre-compiled versions of all contracts. You are only deploying the optimized versions of the following contracts:

• ConduitController
• Consideration
• TransferHelper

For all other contracts, e.g. CriteriaResolver and AdvancedOrder, you are using the normal deployment flow Contract c = new Contract(), which I think means you are compiling the said contracts on the fly.

My question is why? Why are you selectively deploying a few select pre-compiled contracts, and not all? If I understand this correctly, the optimized Foundry profile will automatically compile all contracts, including those who you deploy using the normal flow.

Is it that the chosen contracts are particularly large, and it's them who caused most of the lag when running the tests with --via-ir, but you'd rather keep stack+debug traces for all other contracts, because their compilation time is reasonable?

Whatever the case, I think that it would be worth it to clarify this in the README for future contributors!

[emo-eth]

Hey Paul,
For the latest approaches, make sure you’re taking a look at the 1.2 branch here: https://github.com/ProjectOpenSea/seaport/tree/1.2 (though read me and documentation still need to be updated). Most of the relevant stuff lives in foundry.toml and BaseConsiderationTest (and the GH actions test yaml config).

In the full CI workflow, we pre-compile the optimized byte code for the smart contracts and test against them using the test profile for the three you mentioned. It’s possible there are some places we are still normally deploying the code (SignatureVerifier is an interesting example, as the test contract inherits the signature verification logic directly).
However, the examples you mentioned are structs (note that they are not instantiated with the new keyword) - so there’s no need to pre-compile them, as they’ll be passed as arguments to the core smart contracts.

Thanks for taking a look! And I’ll work on getting the documentation updated for the 1.2 release.

[PaulRBerg]

Thanks for the quick reply, James! I took a look at the 1.2 branch and it all makes sense to me now. I confess that I was a bit unsure what you meant by "conditional deployment" while I was looking at the main branch.

As a side note - this is a perfect example of confirmation bias, where I went looking for what I wanted to see (contracts deployed via the usual way) versus what actually were there (structs).

[PaulRBerg]

I just found another genuine case of a normal deployment flow, that is, the Merkle contract:

seaport/test/foundry/FulfillAdvancedOrderCriteria.t.sol

Line 19 in 188efe8

Merkle merkle = new Merkle();

But of course, the reason in this case is that Merkle is not imported in the contracts directory, so it has to be imported the usual way in the tests (it cannot be deployed as a precompiled contract).

But there are also the TestERC20, TestERC721, TestERC1155, and PreapprovedERC721 contracts:

• seaport/test/foundry/utils/TestTokenMinter.sol

  Line 227 in 188efe8

  token1 = new TestERC20();

• seaport/test/foundry/utils/TestTokenMinter.sol

  Line 230 in 188efe8

  test721_1 = new TestERC721();

• seaport/test/foundry/utils/TestTokenMinter.sol

  Line 233 in 188efe8

  test1155_1 = new TestERC1155();

• seaport/test/foundry/utils/TestTokenMinter.sol

  Line 236 in 188efe8

  preapproved721 = new PreapprovedERC721(preapprovals);

And also the TestZone:

• seaport/test/foundry/zone/PostFulfillmentCheck.t.sol

  Line 31 in 188efe8

  TestZone zone = new TestZone();

For which, I suppose, the same rationale that these contracts are not found in optimized-out applies, so they have to necessarily be compiled in the tests.

[emo-eth]

@PaulRBerg We only precompile the contracts we’ll actually be deploying, to make sure the runtime bytecode we’re testing matches what we’ll be putting on mainnet - there have been various bugs related to the IR pipeline in the past, so we want to be extra sure. The helpers are of less consequence, since they won’t actually be used by end-users.

[PaulRBerg]

Thank you, again, for explaining. All clear now.