Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughRefactors wallet and chain initialization by splitting Changes
Sequence Diagram(s)sequenceDiagram
participant Script as Script
participant Chain as Chain (baseSetup)
participant Transport as Transport
participant Public as PublicClient
participant WalletMaker as makeWallet
participant TDEWallet as tdeDisburser
participant TrackerWallet as trackerDisburser
participant Contracts as Contracts (TDE/Tracker/Token)
Script->>Chain: call baseSetup(chainId, rpcUrl)
Chain->>Transport: init transport
Chain->>Public: init publicClient
Script->>WalletMaker: makeWallet(chain, transport, TDE_KEY)
WalletMaker->>TDEWallet: return { account, walletClient }
Script->>WalletMaker: makeWallet(chain, transport, TRACKER_KEY)
WalletMaker->>TrackerWallet: return { account, walletClient }
Script->>Contracts: bind contracts using TDEWallet.walletClient / TrackerWallet.walletClient
Script->>Contracts: perform allowance/transfer/balance checks using respective disburser addresses
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@script/initial-distribution/src/cca.ts`:
- Around line 78-83: The assertion is reading the disburser from trackerContract
by mistake; change the source of the on-chain value to the TDE disbursement
contract (i.e., call the TDEDisbursement contract's read.disburser() rather than
trackerContract.read.disburser()) so that onChainTDEDisburser is the actual
on-chain disburser for comparison against tdeDisburser.address in the assertion.
In `@script/initial-distribution/src/chains.ts`:
- Around line 51-59: chainSetup duplicates baseSetup logic; change chainSetup to
delegate to baseSetup to avoid drift—inside the chainSetup function call and
return the result of baseSetup (preserving the returned shape { chain,
transport, publicClient }); ensure baseSetup is referenced (and imported if
necessary) and remove the duplicated
resolveChain/http/createPublicClient/validateRpcChainId steps from chainSetup so
there is a single source of truth in baseSetup.
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (5)
script/initial-distribution/src/cca.tsscript/initial-distribution/src/chains.tsscript/initial-distribution/src/claimAllBids.tsscript/initial-distribution/src/tde.tsscript/initial-distribution/src/tdeSetup.ts
📜 Review details
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: pkoch
Repo: idos-network/contracts PR: 13
File: src/WhaleDisburser.sol:23-28
Timestamp: 2026-02-23T19:57:43.946Z
Learning: In src/WhaleDisburser.sol, the disburse function intentionally has no access control. Anyone can call disburse(IERC20 token, address beneficiary, uint256 totalAmount, uint64 vestingStart), which is by design. The function uses safeTransferFrom(msg.sender, ...) so it only pulls from the caller's own balance, and no external funds are at risk.
📚 Learning: 2026-03-01T13:52:37.088Z
Learnt from: pkoch
Repo: idos-network/contracts PR: 19
File: script/initial-distribution/src/tde.ts:38-48
Timestamp: 2026-03-01T13:52:37.088Z
Learning: In script/initial-distribution, when using getContract({ client: walletClient }), it is possible to call read.* methods on the resulting contract even if walletClient is not explicitly extended with publicActions (as observed in tde.ts and cca.ts with viem 2.23.x). Treat this as an intentional pattern to verify: - confirm that the behavior is supported by the specific viem version and contract bindings, - document any assumptions about publicActions not being required for read operations, - ensure there are no security implications (e.g., unintended read access) and that access controls are still correct, - consider adding comments or tests that demonstrate expected usage and edge cases, and - if this behavior is relied upon in other files, apply the same pattern consistently within the script/initial-distribution directory.
Applied to files:
script/initial-distribution/src/claimAllBids.tsscript/initial-distribution/src/cca.tsscript/initial-distribution/src/tde.tsscript/initial-distribution/src/chains.tsscript/initial-distribution/src/tdeSetup.ts
📚 Learning: 2026-02-23T19:57:43.946Z
Learnt from: pkoch
Repo: idos-network/contracts PR: 13
File: src/WhaleDisburser.sol:23-28
Timestamp: 2026-02-23T19:57:43.946Z
Learning: In src/WhaleDisburser.sol, the disburse function intentionally has no access control. Anyone can call disburse(IERC20 token, address beneficiary, uint256 totalAmount, uint64 vestingStart), which is by design. The function uses safeTransferFrom(msg.sender, ...) so it only pulls from the caller's own balance, and no external funds are at risk.
Applied to files:
script/initial-distribution/src/cca.tsscript/initial-distribution/src/tde.ts
🧬 Code graph analysis (3)
script/initial-distribution/src/claimAllBids.ts (3)
script/initial-distribution/src/lib.ts (2)
ensureHex(10-16)requireEnv(3-7)script/initial-distribution/src/chains.ts (2)
baseSetup(37-43)makeWallet(45-49)script/initial-distribution/src/abis.ts (1)
ccaAbi(68-201)
script/initial-distribution/src/cca.ts (3)
script/initial-distribution/src/lib.ts (3)
ensureHex(10-16)requireEnv(3-7)assertCondition(57-59)script/initial-distribution/src/chains.ts (2)
baseSetup(37-43)makeWallet(45-49)script/initial-distribution/src/abis.ts (2)
tdeDisbursementAbi(203-255)trackerAbi(3-66)
script/initial-distribution/src/tdeSetup.ts (3)
script/initial-distribution/src/lib.ts (3)
ensureHex(10-16)requireEnv(3-7)iso8601ToTimestamp(44-51)script/initial-distribution/src/chains.ts (2)
chainSetup(51-59)makeWallet(45-49)script/initial-distribution/src/batch.ts (1)
BatchCallerConfig(38-42)
🔇 Additional comments (6)
script/initial-distribution/src/claimAllBids.ts (2)
27-37: Role-specific tracker disburser wiring looks correct.The migration to
TRACKER_DISBURSER_PRIVATE_KEYplusbaseSetup/makeWalletcleanly aligns this script with the separate-disburser model.
33-37: 🧹 Nitpick | 🔵 TrivialUse explicit
{ public, wallet }client wiring for mixed read/write contract usage.This contract is used for both
read.*andwrite.*operations with only a wallet client. Per viem best practices, explicitly provide both clients:{ public: publicClient, wallet: walletClient }.♻️ Proposed change
const ccaContract = getContract({ address: CCA_ADDRESS, abi: ccaAbi, - client: trackerDisburser.walletClient, + client: { + public: publicClient, + wallet: trackerDisburser.walletClient, + }, });⛔ Skipped due to learnings
Learnt from: pkoch Repo: idos-network/contracts PR: 19 File: script/initial-distribution/src/tde.ts:38-48 Timestamp: 2026-03-01T13:52:37.088Z Learning: In the idos-network/contracts repository, the team uses `getContract({ client: walletClient })` and successfully calls `.read.*` methods on the resulting contract instances in viem 2.23.x, even though the walletClient is not explicitly extended with publicActions. This pattern is used throughout script/initial-distribution/src/ (tde.ts, cca.ts).script/initial-distribution/src/tde.ts (1)
49-52: Allowance checks are now aligned to the correct signer address.Using
tdeDisburser.addresshere avoids allowance mismatches across disburser wallets.script/initial-distribution/src/chains.ts (1)
37-49:baseSetup+makeWalletsplit is a solid refactor.This separation improves composability for multi-wallet flows and matches the PR objective.
script/initial-distribution/src/tdeSetup.ts (1)
27-49: TDE disburser propagation is consistent across setup outputs and clients.
batchConfig,tdeDisbursementContract, andtokenContractnow all usetdeDisburser.walletClient, which keeps signer identity coherent.script/initial-distribution/src/cca.ts (1)
34-60: Role-specific wallet binding across contracts is well-structured.Using
tdeDisburser.walletClientfor token/TDE writes andtrackerDisburser.walletClientfor tracker writes is the right split.
pkoch
force-pushed
the
feat/separate-disbursers
branch
2 times, most recently
from
17a3781 to
7855e40
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
script/initial-distribution/src/chains.ts (1)
51-59: 🧹 Nitpick | 🔵 TrivialDeduplicate
chainSetupby delegating tobaseSetup.
chainSetupre-implements the same logic asbaseSetup, creating maintenance overhead and divergence risk.♻️ Proposed fix
export async function chainSetup(chainId: string, rpcUrl: string) { - const chain = resolveChain(chainId); - const transport = http(rpcUrl); - const publicClient = createPublicClient({ chain, transport }); - - await validateRpcChainId(publicClient, chain); - - return { chain, transport, publicClient }; + return baseSetup(chainId, rpcUrl); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@script/initial-distribution/src/chains.ts` around lines 51 - 59, chainSetup duplicates baseSetup logic; change chainSetup to call and return the result of baseSetup instead of re-implementing transport/publicClient creation and chain resolution. Replace the body of chainSetup to delegate to baseSetup(chainId, rpcUrl) (and, if needed, keep or remove validateRpcChainId depending on whether baseSetup already performs it) so that chainSetup simply returns the { chain, transport, publicClient } produced by baseSetup; reference the functions chainSetup and baseSetup when making this change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@script/initial-distribution/src/chains.ts`:
- Line 2: Remove the unused type import `Address` from the import list in this
file (the `Address` symbol is not referenced anywhere in chains.ts); update the
import statement to omit `Address` so only used types/values are imported to
avoid dead code and linter errors.
In `@script/initial-distribution/src/tde.ts`:
- Line 51: The code incorrectly accesses tdeDisburser.address — update any usage
to access the returned account object from makeWallet by replacing
tdeDisburser.address with tdeDisburser.account.address (or destructure account
from makeWallet earlier and use account.address); ensure references to
tdeDisburser in this file (including where tdeDisburser is created by
makeWallet) consistently use the account.address property so the runtime no
longer sees undefined.
In `@script/initial-distribution/src/tdeSetup.ts`:
- Around line 27-28: Replace the call to chainSetup with baseSetup for
consistency: in tdeSetup.ts swap the line using chainSetup(CHAIN_ID, RPC_URL) to
use baseSetup(CHAIN_ID, RPC_URL), keeping the destructuring { chain,
publicClient, transport } and then continue creating the wallet with
makeWallet(chain, transport, TDE_DISBURSER_PRIVATE_KEY); ensure you import
baseSetup if not already and remove or update any unused chainSetup import/usage
to match other files (e.g., cca.ts, claimAllBids.ts).
---
Duplicate comments:
In `@script/initial-distribution/src/chains.ts`:
- Around line 51-59: chainSetup duplicates baseSetup logic; change chainSetup to
call and return the result of baseSetup instead of re-implementing
transport/publicClient creation and chain resolution. Replace the body of
chainSetup to delegate to baseSetup(chainId, rpcUrl) (and, if needed, keep or
remove validateRpcChainId depending on whether baseSetup already performs it) so
that chainSetup simply returns the { chain, transport, publicClient } produced
by baseSetup; reference the functions chainSetup and baseSetup when making this
change.
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (6)
script/initial-distribution/src/abis.tsscript/initial-distribution/src/cca.tsscript/initial-distribution/src/chains.tsscript/initial-distribution/src/claimAllBids.tsscript/initial-distribution/src/tde.tsscript/initial-distribution/src/tdeSetup.ts
📜 Review details
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: pkoch
Repo: idos-network/contracts PR: 13
File: src/WhaleDisburser.sol:23-28
Timestamp: 2026-02-23T19:57:43.946Z
Learning: In src/WhaleDisburser.sol, the disburse function intentionally has no access control. Anyone can call disburse(IERC20 token, address beneficiary, uint256 totalAmount, uint64 vestingStart), which is by design. The function uses safeTransferFrom(msg.sender, ...) so it only pulls from the caller's own balance, and no external funds are at risk.
📚 Learning: 2026-03-01T13:52:37.088Z
Learnt from: pkoch
Repo: idos-network/contracts PR: 19
File: script/initial-distribution/src/tde.ts:38-48
Timestamp: 2026-03-01T13:52:37.088Z
Learning: In script/initial-distribution, when using getContract({ client: walletClient }), it is possible to call read.* methods on the resulting contract even if walletClient is not explicitly extended with publicActions (as observed in tde.ts and cca.ts with viem 2.23.x). Treat this as an intentional pattern to verify: - confirm that the behavior is supported by the specific viem version and contract bindings, - document any assumptions about publicActions not being required for read operations, - ensure there are no security implications (e.g., unintended read access) and that access controls are still correct, - consider adding comments or tests that demonstrate expected usage and edge cases, and - if this behavior is relied upon in other files, apply the same pattern consistently within the script/initial-distribution directory.
Applied to files:
script/initial-distribution/src/claimAllBids.tsscript/initial-distribution/src/chains.tsscript/initial-distribution/src/cca.tsscript/initial-distribution/src/tdeSetup.tsscript/initial-distribution/src/abis.tsscript/initial-distribution/src/tde.ts
📚 Learning: 2026-02-23T19:57:43.946Z
Learnt from: pkoch
Repo: idos-network/contracts PR: 13
File: src/WhaleDisburser.sol:23-28
Timestamp: 2026-02-23T19:57:43.946Z
Learning: In src/WhaleDisburser.sol, the disburse function intentionally has no access control. Anyone can call disburse(IERC20 token, address beneficiary, uint256 totalAmount, uint64 vestingStart), which is by design. The function uses safeTransferFrom(msg.sender, ...) so it only pulls from the caller's own balance, and no external funds are at risk.
Applied to files:
script/initial-distribution/src/cca.tsscript/initial-distribution/src/abis.tsscript/initial-distribution/src/tde.ts
🧬 Code graph analysis (1)
script/initial-distribution/src/claimAllBids.ts (2)
script/initial-distribution/src/lib.ts (2)
ensureHex(10-16)requireEnv(3-7)script/initial-distribution/src/chains.ts (2)
baseSetup(37-43)makeWallet(45-49)
🔇 Additional comments (17)
script/initial-distribution/src/chains.ts (2)
37-43: LGTM!Clean separation of concerns:
baseSetupproperly initializes the chain infrastructure (chain resolution, transport, public client) and validates the RPC chain ID before returning.
45-49: LGTM!
makeWalletis a clean, synchronous helper that creates a wallet from a private key. Returning bothaccountandwalletClientprovides flexibility for callers needing the address (account.address) or client operations.script/initial-distribution/src/tde.ts (1)
14-23: LGTM on destructuring update.The transition from
accounttotdeDisburserin the destructuring is correct and aligns with the new wallet abstraction pattern.script/initial-distribution/src/claimAllBids.ts (3)
14-14: LGTM!Correctly imports
baseSetupandmakeWalletfrom the updated chains module.
27-31: LGTM!Clean initialization pattern: environment variable renamed appropriately, and wallet created using the new
makeWallethelper.
33-37: LGTM!Contract client correctly bound to
trackerDisburser.walletClient. Based on learnings, usinggetContract({ client: walletClient })for read operations is an intentional pattern verified to work with viem 2.23.x.script/initial-distribution/src/tdeSetup.ts (2)
36-46: LGTM!Contract clients correctly bound to
tdeDisburser.walletClient. Based on learnings, this pattern allows callingread.*methods even without explicitly extending withpublicActionsin viem 2.23.x.
48-57: LGTM!Return object correctly includes
tdeDisburserand all necessary configuration for downstream consumers.script/initial-distribution/src/abis.ts (4)
203-210: LGTM!The
DISBURSERview function is correctly defined to read the on-chain disburser address, enabling the verification incca.ts.
218-228: LGTM!
VESTING_PARAMS_FOR_MODALITYis properly defined as a pure function returning vesting parameters.
263-273: LGTM!The
Disbursedevent includes all necessary fields for tracking disbursements: beneficiary (indexed for filtering), transferTarget, amount, and modality.
274-288: LGTM!Comprehensive error definitions provide clear failure modes for input validation and access control.
script/initial-distribution/src/cca.ts (5)
34-37: LGTM!Clean dual-wallet initialization:
baseSetupfor chain infrastructure, then separatemakeWalletcalls for TDE and tracker disbursers with their respective private keys.
45-61: LGTM!Contract clients are correctly bound to their respective wallet clients:
soldTokenContractandtdeDisbursementContractusetdeDisburser.walletClient, whiletrackerContractusestrackerDisburser.walletClient.
71-83: LGTM!On-chain disburser verification correctly validates both disbursers:
- Reads
disburser()from tracker contract and validates againsttrackerDisburser.account.address- Reads
DISBURSER()from TDE contract and validates againsttdeDisburser.account.addressThis ensures the configured wallets match the on-chain authorized disbursers before proceeding.
169-180: LGTM!
ensureTdeAllowancecorrectly usestdeDisburser.account.addresswhen checking and setting token allowance for the TDE disbursement contract.
330-348: LGTM!Final balance check and sweep logic correctly uses
tdeDisburser.account.addressfor balance reads and the post-sweep assertion.
pkoch
force-pushed
the
feat/separate-disbursers
branch
from
7855e40 to
3f86ded
Compare
1 participant
Summary by CodeRabbit
Refactor
New Features