Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Agree on deprecating dapp tools #398

Open
SidestreamColdMelon opened this issue Mar 4, 2024 · 8 comments
Open

Agree on deprecating dapp tools #398

SidestreamColdMelon opened this issue Mar 4, 2024 · 8 comments
Assignees

Comments

@SidestreamColdMelon
Copy link
Contributor

Discuss strategy of dapp tools deprecation and if we want to do it.

One counter argument: having dapp tools and foundry cross checking the output binary is better than having a single tool to circumvent supply chain attacks. In this case, we can make foundry the main development+deployment tool and use dapp-tools for crosschecking (and not the way around).

@SidestreamColdMelon
Copy link
Contributor Author

SidestreamColdMelon commented May 6, 2024

Based on the above, the following would be a proposed plan to migrate from dapp-tools:

  • Remove dependency on dapp-tools for flattening the source code
  • Replace all occurrences of seth and hevm commands with alternatives (foundry/python/node-based)
  • Remove dependency on ./out/dapp.sol.json file
  • Replace dapp-tools with foundry for compilation and deployment
  • Extend checklists to enforce using stable foundry version and document used version / logs
  • Add dockerfile + CI workflow to run tests using dapp-tools

@SidestreamColdMelon SidestreamColdMelon self-assigned this May 6, 2024
@amusingaxl
Copy link
Contributor

I agree with the changes.
The only potentially challenging part would be setting up dapp tools in CI for cross-checking.

With that said, dapp tools being basically abandonware nowadays adds more risks than benefits.

Supply chain attacks would most likely be very hard to catch, since MakerDAO is probably the only relevant org that keeps using it. If we do not very thoroughly check the sources from which we pull the executables every single time, it would be very hard to catch a problem. However, this seems like a huge operational burden.

An alternative would be to port the "immutable" part of the flow (i.e.: base tests) to another tool that is still under active maintenance, such as Hardhat, and perform the cross-checks with it.

@SidestreamColdMelon
Copy link
Contributor Author

dapp tools being basically abandonware nowadays adds more risks than benefits.

I don't think it adds any risks if it's executed within a CI / docker environment and is no longer used for deployment (or otherwise have access to private keys) – this way, dapp-tools would only get access to the spell code which is already available to everyone. The only thing approach adds:

  • Overhead for the one-time setup of the environment (creating a docker file + CI config)
  • Overhead for keeping test configuration compatible with both tools (this is kind of what we are doing already by running tests with foundry and deploying with dapp tools)

This approach removes the risk of either foundry or dapp-tools going rogue, but not saving us from other supply chain attacks (eg patched solc compiler) – but that wouldn't be achievable even with the proposed Hardhat-based tests.

An alternative would be to port the "immutable" part of the flow (i.e.: base tests) to another tool that is still under active maintenance, such as Hardhat, and perform the cross-checks with it.

I see that this brings a huge overhead on keeping two source codes written in different languages in sync. I can also imagine that hardhat-based tests would be a lot slower.

@amusingaxl
Copy link
Contributor

Going back to this after a while.

  • Extend checklists to enforce using stable foundry version and document used version / logs

This one can be tricky. There's no "stability" in Foundry development yet, it follows the nightly releases approach, so it would be extremely hard to pinpoint a "safe" version.

  • Add dockerfile + CI workflow to run tests using dapp-tools

This is also tricky because the current tests depend on Foundry and don't work on Dapp Tools anymore because we are using Foundry-specific features.


In the end of the day, I become more and more skeptical of the benefits that cross-checking brings. I am not aware of any other project in this industry that does that.

If we fear that supply chain attacks might occur, we can come up with a "dev manual" on how to deal with this (i.e.: only install Foundry from official sources), so we can minimize the risk.

@SidestreamColdMelon
Copy link
Contributor Author

Thanks for reviving this conversation!

  • Extend checklists to enforce using stable foundry version and document used version / logs

This one can be tricky. There's no "stability" in Foundry development yet, it follows the nightly releases approach, so it would be extremely hard to pinpoint a "safe" version.

Looking at how situation went with the last big supply chain attack using Ledger library, it was detected by the community fairly quickly. So to be on the safe side, we can add a requirement to checklist to use a foundry version that is, for example, exactly one week old + document which exact version was used. Instead of requiring to use foundryup which pulls the latest version that can be baked a minute ago and immediately have access to our private env.

  • Add dockerfile + CI workflow to run tests using dapp-tools

This is also tricky because the current tests depend on Foundry and don't work on Dapp Tools anymore because we are using Foundry-specific features.

I agree that it can be a problem. As proposed in the last call, we can then use dapp-tools to only run a test similar to testBytecodeMatches (and only inside the CI).

I am not aware of any other project in this industry that does that.

I don't think it's a valid argument. No one in the industry is using checklists (to my knowledge), but it's a simple and powerful tool used in other industries (namely aviation, healthcare) and we're already using it. Same with cross-compilation: it's a simple technique and we're already using it, so let's maintain it, unless there is a very substantial effort required for this to happen (which is also a valid argument). So to push the agreement forward: I agree to drop cross-compilation, if we try and fail to archive it within reasonable amount of time.

@amusingaxl
Copy link
Contributor

So to be on the safe side, we can add a requirement to checklist to use a foundry version that is, for example, exactly one week old + document which exact version was used. Instead of requiring to use foundryup which pulls the latest version that can be baked a minute ago and immediately have access to our private env.

Agreed. Let's amend the checklists then.

However, I'm not entirely sure how to enforce this 7 days grace period. What happens if someone called foundryup recently and need to revert to a previous version? Is that even possible?

Same with cross-compilation: it's a simple technique and we're already using it, so let's maintain it, unless there is a very substantial effort required for this to happen (which is also a valid argument)

We're not exactly doing cross-compilation here. Both Foundry and Dapp Tools use solc under the hood to compile the contracts.

The only gains the cross-checking brings is if any of those tools manage to successfully replace a valid solc build with a rogue one.

@SidestreamColdMelon
Copy link
Contributor Author

However, I'm not entirely sure how to enforce this 7 days grace period. What happens if someone called foundryup recently and need to revert to a previous version? Is that even possible?

Yes, it's possible to specify exact foundry version to install using -v/--version flag. So I would propose the following flow:

foundryup: installed - forge 0.2.0 (4817280 2024-11-13T00:20:59.079036000Z)
foundryup: installed - cast 0.2.0 (4817280 2024-11-13T00:20:59.133279000Z)
foundryup: installed - anvil 0.2.0 (4817280 2024-11-13T00:20:59.147349000Z)
foundryup: installed - chisel 0.2.0 (4817280 2024-11-13T00:20:59.119584000Z)

I can add this to checklists. I also think the crafter should post this comment as well as the reviewers (which is potentially even more important, since the deployment is done by the crafter).

We're not exactly doing cross-compilation here.

Technically yes. If solc itself is compromised, that wouldn't be detected in the proposed solution. But we're not updating solc version every two weeks. In fact, we didn't bump the version with which spells are being compiled for at least 2 years. So this risk is minimal. It's more likely that the supply chain attack will target dapp-tools or foundry.

@amusingaxl
Copy link
Contributor

Sounds good.
Would you please update the checklists then?
I'll look into how we can run testBytecodeMatches with dapp tools from CI.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants