-
Notifications
You must be signed in to change notification settings - Fork 2
Feat/deploy base sepolia #67
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
base: master
Are you sure you want to change the base?
Conversation
WalkthroughAdds Base Sepolia network support and Basescan config; updates Etherscan keys. Replaces Kovan deploy script with Base Sepolia. Parameterizes deploy-market-view by chain. Introduces a new deploy-v2 script deploying and verifying multiple contracts. Modifies main deploy script with chain 84532 params and conditional CurateProxy/arbitrator handling and verification tweaks. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer
participant HH as Hardhat Script (scripts/deploy.js)
participant Net as Network (RPC)
participant Cur as CurateProxy
participant Desc as BetNFTDescriptor
participant Fac as MarketFactory
Dev->>HH: run --network base-sepolia
HH->>Net: get chainId
alt curate configured
HH->>Net: deploy CurateProxy
activate Cur
Cur-->>HH: address
deactivate Cur
else curate not configured
Note over HH: Skip CurateProxy
end
HH->>Net: deploy BetNFTDescriptor (curate ? Cur.address : AddressZero)
activate Desc
Desc-->>HH: address
deactivate Desc
HH->>Net: deploy MarketFactory (arbitrator || AddressZero, governor, treasury,…)
activate Fac
Fac-->>HH: address
deactivate Fac
par verify contracts
HH->>Net: verify BetNFTDescriptor (args per curate presence)
HH->>Net: verify MarketFactory (arbitrator may be AddressZero)
opt curate configured
HH->>Net: verify CurateProxy
end
end
sequenceDiagram
autonumber
actor Dev as Developer
participant HH as Hardhat Script (scripts/deploy-v2.js)
participant Net as Network (RPC)
participant LP as LiquidityPool
participant LF as LiquidityFactory
participant KV as KeyValue
participant MF2 as MarketFactoryV2
Dev->>HH: run deploy-v2
HH->>Net: get chainId & params (realityRegistry, marketFactory, governor)
HH->>Net: deploy LiquidityPool
LP-->>HH: address
HH->>Net: deploy LiquidityFactory (marketFactory, LP, governor)
LF-->>HH: address
HH->>Net: deploy KeyValue
KV-->>HH: address
HH->>Net: deploy MarketFactoryV2 (realityRegistry, KV, marketFactory, LF)
MF2-->>HH: address
par verify
HH->>Net: verify LiquidityPool
HH->>Net: verify KeyValue
HH->>Net: verify LiquidityFactory (ctor args)
HH->>Net: verify MarketFactoryV2 (ctor args)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
scripts/deploy.js (1)
38-124: Fix the curateProxy shadowing bugWithin Lines 39-43
const curateProxy = …shadows the outerlet, so the outer variable never gets assigned. The script then crashes when it tries to readcurateProxy.address, and BetNFTDescriptor is always initialised withAddressZero. Assign to the outer variable instead.- let curateProxy; + let curateProxy; if (params[chainId].curate) { const CurateProxy = await ethers.getContractFactory("CurateProxy"); - const curateProxy = await CurateProxy.deploy(params[chainId].curate); - await curateProxy.deployed(); + curateProxy = await CurateProxy.deploy(params[chainId].curate); + await curateProxy.deployed(); }
🧹 Nitpick comments (2)
scripts/deploy-market-view.js (1)
4-32: Validate the chain configuration before using it
params[chainId]is used immediately, so an unsupported network will trigger aCannot read properties of undefinedlater on. Add an explicit guard near Line 19 to bail out with a clear message.const chainId = hre.network.config.chainId; + if (!params[chainId]) { + throw new Error(`Unsupported chainId ${chainId} for MarketView deployment`); + }scripts/deploy-v2.js (1)
5-101: Guard against missing params entriesLine 19 reads
params[chainId]without checking for existence. On an unsupported network this script will crash deep into deployment. Add the same fail-fast guard used elsewhere so users get a clear error.const chainId = hre.network.config.chainId; + if (!params[chainId]) { + throw new Error(`Unsupported chainId ${chainId} for deploy-v2`); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (5)
hardhat.config.js(3 hunks)package.json(1 hunks)scripts/deploy-market-view.js(4 hunks)scripts/deploy-v2.js(1 hunks)scripts/deploy.js(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
scripts/deploy.js (1)
test/factory.js (5)
curateProxy(87-87)CurateProxy(86-86)ethers(3-3)BetNFTDescriptor(90-90)betNFTDescriptor(36-36)
| network: "base-sepolia", | ||
| chainId: 84532, | ||
| urls: { | ||
| // 3) Select to what explorer verify the contracts | ||
| // BaseScan | ||
| apiURL: "https://api.etherscan.io/v2/api?chainid=84532", | ||
| browserURL: "https://sepolia.basescan.org/", | ||
| }, |
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.
Point the Base Sepolia explorer config at BaseScan
Line 108 uses https://api.etherscan.io/v2/api?chainid=84532, which is not the Base Sepolia explorer endpoint. Hardhat verification will fail against that URL. Use the BaseScan API instead.
- apiURL: "https://api.etherscan.io/v2/api?chainid=84532",
+ apiURL: "https://api-sepolia.basescan.org/api",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| network: "base-sepolia", | |
| chainId: 84532, | |
| urls: { | |
| // 3) Select to what explorer verify the contracts | |
| // BaseScan | |
| apiURL: "https://api.etherscan.io/v2/api?chainid=84532", | |
| browserURL: "https://sepolia.basescan.org/", | |
| }, | |
| network: "base-sepolia", | |
| chainId: 84532, | |
| urls: { | |
| // 3) Select to what explorer verify the contracts | |
| // BaseScan | |
| apiURL: "https://api-sepolia.basescan.org/api", | |
| browserURL: "https://sepolia.basescan.org/", | |
| }, |
🤖 Prompt for AI Agents
In hardhat.config.js around lines 103 to 110, the explorer apiURL is incorrectly
set to Etherscan (https://api.etherscan.io/...) which will fail for Base
Sepolia; update the apiURL to BaseScan's endpoint (e.g.
https://api.basescan.org/api?chainid=84532) and ensure the project uses the
correct explorer API key variable (e.g. BASESCAN_API_KEY) in the
hardhat-etherscan configuration; verify browserURL stays pointing at the Base
Sepolia explorer and that the chainId matches the network.
| "scripts": { | ||
| "prettier": "prettier --write 'contracts/**/*.sol'", | ||
| "deploy:kovan": "npx hardhat run --network kovan scripts/deploy.js", | ||
| "deploy:baseSepolia": "npx hardhat run --network 'base-sepolia' scripts/deploy.js", |
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.
Drop the single quotes from the network name
On Line 9 the script passes 'base-sepolia'. When run on Windows the quotes are preserved, so Hardhat looks for a network called 'base-sepolia' and the deployment fails. Please remove the quotes so the command works cross-platform.
- "deploy:baseSepolia": "npx hardhat run --network 'base-sepolia' scripts/deploy.js",
+ "deploy:baseSepolia": "npx hardhat run --network base-sepolia scripts/deploy.js",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "deploy:baseSepolia": "npx hardhat run --network 'base-sepolia' scripts/deploy.js", | |
| "deploy:baseSepolia": "npx hardhat run --network base-sepolia scripts/deploy.js", |
🤖 Prompt for AI Agents
In package.json around line 9, the npm script "deploy:baseSepolia" uses quoted
network name 'base-sepolia' which on Windows is passed verbatim to Hardhat;
remove the single quotes so the script reads the network name without quotes
(i.e., use base-sepolia) to ensure cross-platform correctness when running npx
hardhat --network.
Support to deploy at base sepolia
Summary by CodeRabbit
New Features
Refactor
Chores