Skip to content

Comments

Migrate deployment to use cannon#257

Open
kaze-cow wants to merge 18 commits intonew-chain-deploymentsfrom
cannon3
Open

Migrate deployment to use cannon#257
kaze-cow wants to merge 18 commits intonew-chain-deploymentsfrom
cannon3

Conversation

@kaze-cow
Copy link
Contributor

@kaze-cow kaze-cow commented Feb 18, 2026

Description

In order to improve multi-network multi-repository deployments of CoW protocol, we are switching to using Cannon. This retains the existing deployment system. Unfortunately, many of the smart contracts have been updated (causing changes in the deployed contract address of the settlement contract), so techincally these also have to be referted in order for the correct package version to be generated.

Considering the smart contract changes we ultimately want to have in the next settlement contract, it seems smart to move them into a new dev branch which we use as the branch where PRs get merged until we are ready to deploy settlement v1.5

It also includes all the appropriate documentation for continued maintenance and usage of cannon.

Test Plan

Similar to other cannon releases:

  • Install dependencies (yarn install)
  • Run cannon package build (yarn hardhat cannon:build --network cannon --wipe)
  • Verify running yarn record-cannon results in no substantial changes to the recorded package manifest (some changes are to be expected, such as timestamp and chainDumps will change due to the time changing)
  • verify tests and etc. in the repo are still working as before

Related PRs

This PR is the product of two previous PRs as I experimented with getting a good implementation of Cannon in this repo.

#255 -- previous PR which also removes hardhat-deploy and has much, much more changes. This PR is much more minimal.
#256 -- this one was supposed to be the "minimized" version, but its based off the main branch which doesn't produce the proper artifacts. This PR is based on new-chain-deployments

@kaze-cow kaze-cow requested a review from a team February 18, 2026 09:49
@socket-security
Copy link

socket-security bot commented Feb 18, 2026

All alerts resolved. Learn more about Socket for GitHub.

This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored.

View full report

@kaze-cow kaze-cow marked this pull request as ready for review February 18, 2026 09:59
Copy link
Contributor

@anxolin anxolin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test plan was great, and worked. Only thing, I saw a incoherence with the generated artifact path in the README.

I checked the contract matches, the tests pass, and the main configs. Looks pretty good.

Description of PR was good, but some things that could be better are

  • Title is strange. Should be written like a command you give to the code to change. For example in this case "Migrate deployment to use cannon" or sometihng similar
  • The 2 notes are a bit strange, if CI fails, maybe is good we set it up later so we have an independent PR just for that that is more soped and easier to review and test. This PR doess a lot of things at once. Anyways, im not concerns, so don't split it.
  • The description feels more like "a context" you are giving, which is nice, but I would love to read as the first part of a description a highlight of what I expect to see later in the code (so I understand the scope). Something like, "This PR includes the migration to cannon, and the following are the main changes:" or sth similar

README.md Outdated
NETWORK='rinkeby'
GAS_PRICE_WEI='1000000000'
yarn deploy --network $NETWORK --gasprice $GAS_PRICE_WEI
yarn hardhat cannon:build --network cannon --wipe
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what's the plan for networks.json file? do we keep it? probably nice cause is likely theres many places pointing to it and is convenient, but is also duplicated info, plus can fall out of sync with the cannon/deploy.json

Also, what's the plan with multiple environments? We only have one file cannon/deploy.json

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what's the plan for networks.json file? do we keep it?

I think we should keep it as is for backwards-compatibility reasons, but direct users to refer to the deployments repo for most up-to-date information

Also, what's the plan with multiple environments? We only have one file cannon/deploy.json

thats right. The file that is stored in this repo is the "local network" deployment (chain id 13370), and its designed to serve as an effective "blueprint" for what actually gets deployed on the real networks later. For the multiple networks that will be deployed in the future, those manifests all get stored in the deployments repository.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should keep it as is for backwards-compatibility reasons, but direct users to refer to the deployments repo for most up-to-date information

But if we want to keep it, I guess we will need to do a script or something for generating it automatically?
Otherwise, it will fall out of sync soon.

Maybe we don't need it if we can provide an alternative way to get the addresses. Like pointing to our docs or the deployment github repo or something. I think we should discuss it a bit more what's best here. I'm fine keeping it out of the PR, but I think we will need to follow up on this

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But if we want to keep it, I guess we will need to do a script or something for generating it automatically?
Otherwise, it will fall out of sync soon.

Yea I think its Ok if its OOD as long as we tell users they have to go elsewhere for the most up to date info

@kaze-cow kaze-cow changed the title even more minimal changes for cannon Migrate deployment to use cannon Feb 20, 2026
@kaze-cow kaze-cow requested a review from anxolin February 20, 2026 09:05
Copy link
Contributor

@anxolin anxolin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kaze-cow would it make sense to always create the same NPM scripts (same name) in all projects?

For example, this project usesyarn build to build the contracts.

Given this command to build cannon packages don't change (they have the same netork and params).

What do you think we add some convenient scripts:

  • yarn build:cannon: would run the 2 commands bellow
  • yarn cannon:package: would run yarn hardhat cannon:build --network cannon --wipe
  • yarn cannon:registry: Rename for your record-cannon script

Just a suggestion, maybe there's better naming conventions. The point is to have a simple command that is familiar across projects (always the same)

kaze-cow and others added 2 commits February 20, 2026 21:03
Co-authored-by: Anxo Rodriguez <anxo@cow.fi>
@kaze-cow
Copy link
Contributor Author

What do you think we add some convenient scripts:

yarn build:cannon: would run the 2 commands bellow
yarn cannon:package: would run yarn hardhat cannon:build --network cannon --wipe
yarn cannon:registry: Rename for your record-cannon script

Just a suggestion, maybe there's better naming conventions. The point is to have a simple command that is familiar > across projects (always the same)

I like the naming approach you suggested. currently we have one script record-cannon shared between projects, but this pattern is intuitive, and unifies the build commands between different repositories. Let me go ahead and set that up in this and the other PRs!

@kaze-cow kaze-cow requested a review from anxolin February 20, 2026 12:20
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

Successfully merging this pull request may close these issues.

2 participants