-
Notifications
You must be signed in to change notification settings - Fork 1
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
feat: deploy to Linea #18
Conversation
richard-ramos
commented
Oct 1, 2024
•
edited
Loading
edited
- Splits deployment script in separate deployments per contract (since we might want in the future to deploy different PriceCalculators or WakuRLNV2 contracts).
- Adds https://github.com/Cyfrin/foundry-devops to retrieve the latest address deployed. (Useful when using multiple scripts)
- Adds separate package.json scripts to control the deployment of each individual contract
- Setups the deployment for Linea Sepolia
38181df
to
da65355
Compare
bd9bb06
to
38783cf
Compare
script/Deploy.s.sol
Outdated
|
||
function run() public broadcast returns (address) { | ||
address priceCalcAddr = DevOpsTools.get_most_recent_deployment("LinearPriceCalculator", block.chainid); | ||
address wakuRlnV2ImplAddr = DevOpsTools.get_most_recent_deployment("WakuRlnV2", block.chainid); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is cool, but what happens when you don't have a broadcast
directory with a previous deployment on your machine?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, good point.
I'll change the script to allow the usage of environment variables and/or indicate you want to use the broadcast directory.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change implemented in a5acb1d
Now it will by default use the broadcast dir, unless env variables are used for contracts that were already deployed.
6c9ce4d
to
a5acb1d
Compare
2b1600a
to
6db38e3
Compare
I think deployment on Linea testnet is fine. However, we should have a specific issue to plan and track switching TWN from Ethereum Sepolia to Linea testnet. cc @jm-clius @s-tikhomirov |
Spoke briefly about this with @s-tikhomirov. We'll create an issue in pm-repo that tracks the steps to get to Linea (mainnet). This will require some thinking, but the steps may look something like this:
|
I've deployed the contracts including changes proposed on PR #22 : 0xb9cd878c90e49f797b4431fbf4fb333108cb90e6 For ease of test, I'm using the following test token that can be minted by anyone: cc: @Ivansete-status |
Some suggested next steps set out here: waku-org/pm#258 |
@s-tikhomirov @0x-r4bbit I think this PR should be fine to merge once conflicts are fixed, right? |
src/Membership.sol
Outdated
require(0 < _minMembershipRateLimit); | ||
require(_minMembershipRateLimit <= _maxMembershipRateLimit); | ||
require(_maxMembershipRateLimit <= _maxTotalRateLimit); | ||
require(_activeDurationForNewMemberships > 0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps we should follow the linter's advice and add error messages here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if worth it. These are checks that will be executed only during deployment. It's highly likely that we'll be using values hardcoded in the deploy script that will never hit these requires
(like we do now).