-
-
Notifications
You must be signed in to change notification settings - Fork 1
Add sponsorship #91
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
Add sponsorship #91
Conversation
✅ Deploy Preview for web3-sponsors ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
elimu.ai's mission is to build innovative learning software that empowers out-of-school children to teach themselves basic reading📖, writing✍🏽 and math🔢 within 6 months. WalkthroughRemoves language support from SponsorshipQueue, simplifying constructor and add flow. Updates tests, frontend to pay a fixed estimated cost without language codes. Adds Ignition module for QueueHandler and simplifies SponsorshipQueue module. Updates deployment docs and gas report. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Frontend as Frontend (Add Sponsorship)
participant Wallet as Wallet/Provider
participant SQ as SponsorshipQueue
participant QH as QueueHandler
User->>Frontend: Open Add Sponsorship
Frontend-->>User: Show estimatedCost
User->>Frontend: Click "Send {estimatedCost} ETH"
Frontend->>Wallet: Request tx to SQ.addSponsorship() with value=estimatedCost
Wallet->>SQ: submit transaction
SQ-->>SQ: enqueue Sponsorship {estimatedCost, timestamp, sponsor}
alt Queue processing later
SQ->>QH: processed via QueueHandler (out of scope)
end
SQ-->>Wallet: tx receipt
Wallet-->>Frontend: success
Frontend-->>User: Confirmation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal). Please share your feedback with us on this Discord post. 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: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/contracts/SponsorshipQueue.sol (1)
78-81
: Prefer call over transfer and handle failuretransfer’s 2300 gas stipend is brittle. Use call and check the result.
- payable(distributor).transfer(sponsorship.estimatedCost); + (bool ok, ) = payable(distributor).call{ value: sponsorship.estimatedCost }(""); + require(ok, "Payout failed");
🧹 Nitpick comments (12)
backend/README.md (1)
25-26
: Docs: new QueueHandler deploy steps look good; add clarity on mainnet + resets
- Add a short note that mainnet is not yet deployed and addresses are placeholders to avoid confusion with the chain-1 deployed_addresses.json link.
- Consider adding --reset for first-time or constructor‑changed deployments across all modules to keep artifacts consistent.
- If using --verify, note the need for the Etherscan API key.
Keeping deployment docs crisp helps us deliver on elimu.ai’s mission: build innovative learning software that empowers out-of-school children to teach themselves basic reading📖, writing✍🏽 and math🔢 within 6 months.
Also applies to: 34-34, 43-43
frontend/src/pages/sponsorships/add.tsx (1)
61-69
: Optional: fetch estimatedCost from the contract instead of hard‑codingRead SponsorshipQueue.estimatedCost via a read hook and pass the on‑chain value end‑to‑end to avoid drift with backend/ignition.
backend/contracts/SponsorshipQueue.sol (1)
24-24
: Remove unused errorInvalidLanguageCode is dead code after removing language support.
- error InvalidLanguageCode();
backend/ignition/modules/SponsorshipQueue.ts (1)
9-9
: LGTM—constructor arg matches contract changeConsider sourcing this value from config/env and letting the frontend read it from the contract to keep one source of truth with the UI.
backend/test/QueueHandler.ts (2)
153-153
: Updated addSponsorship usage is correct; consider adding a mismatch-value revert test.If the contract enforces exact msg.value == estimatedCost, add a negative test to prevent regressions.
171-189
: Balance check: prefer explicit address and balance matcher for clarity.Use account2.address with getBalance, or Hardhat’s changeEtherBalances for a crisper assertion (avoids logs/noise).
-const sponsorBalanceBefore = await ethers.provider.getBalance(account2); +const sponsorBalanceBefore = await ethers.provider.getBalance(account2.address); -const sponsorBalanceAfter = await ethers.provider.getBalance(account2); +const sponsorBalanceAfter = await ethers.provider.getBalance(account2.address);Optional (clearer assertion):
-await queueHandler.processQueuePair(); -const sponsorBalanceDiff = sponsorBalanceAfter - sponsorBalanceBefore; -expect(sponsorBalanceDiff).to.equal(hre.ethers.parseUnits("0.02")); +await expect(queueHandler.processQueuePair()) + .to.changeEtherBalances([account2.address], [hre.ethers.parseUnits("0.02")])backend/ignition/modules/QueueHandler.ts (2)
8-15
: Use strict equality and unify address sourcing.Swap == for ===; consider sourcing roles from a JSON (like others) to avoid drift.
-if (network.name == "sepolia") { +if (network.name === "sepolia") { ... -} else if (network.name == "mainnet") { +} else if (network.name === "mainnet") {
5-7
: Console noise in modules.Switch logs to debug or gate via env to keep deploy output clean.
backend/test/SponsorshipQueue.ts (4)
68-69
: Event assertion could be stronger.If the event has args (e.g., sponsor, amount), assert them to catch shape regressions.
80-84
: Prefer getAddress() over target for consistency.Both work in ethers v6, but getAddress() is explicit and future‑proof.
-const contractBalance = await hre.ethers.provider.getBalance(sponsorshipQueue.target); +const contractBalance = await hre.ethers.provider.getBalance(await sponsorshipQueue.getAddress());
72-77
: Remove unused balance logs to keep tests clean.The intermediate balance is only logged; drop it to reduce noise/flakiness in CI logs.
93-102
: Add a negative test for incorrect msg.value.If the contract requires exact estimatedCost, add a revert test with value ≠ estimatedCost.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
backend/README.md
(2 hunks)backend/contracts/SponsorshipQueue.sol
(2 hunks)backend/hardhat-gas-report.md
(3 hunks)backend/ignition/modules/QueueHandler.ts
(1 hunks)backend/ignition/modules/SponsorshipQueue.ts
(1 hunks)backend/test/QueueHandler.ts
(3 hunks)backend/test/SponsorshipQueue.ts
(4 hunks)frontend/src/pages/sponsorships/add.tsx
(5 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-12T06:22:30.517Z
Learnt from: jo-elimu
PR: elimu-ai/web3-sponsors#74
File: backend/ignition/modules/SponsorshipQueue.ts:9-17
Timestamp: 2025-08-12T06:22:30.517Z
Learning: The web3-sponsors project has not deployed to Ethereum Mainnet yet. The placeholder "0x addr mainnet" in backend/ignition/modules/SponsorshipQueue.ts is intentional and appropriate for the current deployment state.
Applied to files:
backend/ignition/modules/SponsorshipQueue.ts
frontend/src/pages/sponsorships/add.tsx
backend/test/QueueHandler.ts
backend/README.md
backend/test/SponsorshipQueue.ts
🧬 Code graph analysis (1)
frontend/src/pages/sponsorships/add.tsx (1)
frontend/src/pages/distributions/add.tsx (2)
SimulateContractButton
(109-141)WriteContractButton
(143-165)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build (windows-latest, 18.x)
- GitHub Check: build (windows-latest, 20.x)
🔇 Additional comments (5)
backend/hardhat-gas-report.md (1)
34-35
: Gas report updated—please confirm it’s generated from this commitNumbers align with the simplified SponsorshipQueue, but please confirm the report was regenerated after these changes (and re‑generate again after the Solidity fixes I’m proposing).
Also applies to: 42-42, 54-54, 56-56, 60-60, 76-78
frontend/src/pages/sponsorships/add.tsx (1)
52-52
: Nice simplification of the flowSwitching to ReadEstimatedCost keeps UX focused and consistent with the backend changes.
backend/ignition/modules/QueueHandler.ts (2)
12-15
: Mainnet placeholder acknowledged.Per prior learning, the “0x addr mainnet” placeholder is intentional until mainnet is supported.
When ready, replace with the address from dao-contracts chain-1 deployed_addresses.json in the sister repo.
1-4
: No action required — resolveJsonModule is enabled.
Verified: tsconfig.json compilerOptions.resolveJsonModule = true (script output: true, true).
elimu.ai's mission is to build innovative learning software that empowers out-of-school children to teach themselves basic reading📖, writing✍🏽 and math🔢 within 6 months.backend/test/QueueHandler.ts (1)
24-27
: Constructor/API alignment confirmed — no stale call sites remain.No legacy two-arg SponsorshipQueue.deploy or addSponsorship(string ...) usages found. Remaining one-arg deploys: backend/test/QueueHandler.ts:26, backend/test/SponsorshipQueue.ts:18.
elimu.ai's mission is to build innovative learning software that empowers out-of-school children to teach themselves basic reading📖, writing✍🏽 and math🔢 within 6 months.
function addSponsorship() public payable { | ||
payable(address(this)).send(estimatedCost); | ||
Sponsorship memory sponsorship = Sponsorship( | ||
msg.value, | ||
languageCode, | ||
estimatedCost, | ||
block.timestamp, | ||
msg.sender | ||
); |
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.
Critical: addSponsorship does not collect ETH; self-send is a no-op and unchecked
- payable(address(this)).send(estimatedCost) attempts to send from the contract to itself, will fail without a payable receive, and is unchecked.
- A sponsor can send 0 and still be enqueued; later payouts will revert due to insufficient balance.
Enforce exact payment and remove the self-send:
- function addSponsorship() public payable {
- payable(address(this)).send(estimatedCost);
+ function addSponsorship() public payable {
+ require(msg.value == estimatedCost, "Incorrect sponsorship amount");
Sponsorship memory sponsorship = Sponsorship(
estimatedCost,
block.timestamp,
msg.sender
);
enqueue(sponsorship);
emit SponsorshipAdded(sponsorship);
}
Optional (outside this hunk): add a receive to accept direct top-ups if desired.
receive() external payable {}
🤖 Prompt for AI Agents
backend/contracts/SponsorshipQueue.sol around lines 49 to 55: remove the
self-send call and enforce that callers actually pay the required amount by
replacing the no-op with a require(msg.value == estimatedCost, "must pay
estimatedCost") (or require(msg.value >= estimatedCost) if overpayments are
allowed and handle change), then use msg.value (or estimatedCost) when
constructing the Sponsorship struct; do not use payable(address(this)).send(...)
and ensure no unchecked transfers occur here; optionally add a receive()
external payable {} elsewhere if you want to accept direct top-ups.
let sponsorshipQueueAddress = ethers.ZeroAddress; | ||
if (network.name == "sepolia") { | ||
sponsorshipQueueAddress = deployed_addresses["SponsorshipQueueModule#SponsorshipQueue"]; | ||
} else if (network.name == "mainnet") { | ||
// TODO: ./ignition/deployments/chain-1/deployed_addresses.json | ||
} | ||
console.log("sponsorshipQueueAddress:", sponsorshipQueueAddress); | ||
|
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.
Avoid ZeroAddress defaults on unsupported networks; fail fast or parameterize.
On “hardhat”/“localhost” this leaves all deps as 0x000… and will deploy a broken QueueHandler.
- let sponsorshipQueueAddress = ethers.ZeroAddress;
- if (network.name == "sepolia") {
+ let sponsorshipQueueAddress = ethers.ZeroAddress;
+ if (network.name === "sepolia") {
sponsorshipQueueAddress = deployed_addresses["SponsorshipQueueModule#SponsorshipQueue"];
- } else if (network.name == "mainnet") {
+ } else if (network.name === "mainnet") {
// TODO: ./ignition/deployments/chain-1/deployed_addresses.json
}
+ if (sponsorshipQueueAddress === ethers.ZeroAddress) {
+ throw new Error(`Missing SponsorshipQueue address for network ${network.name}`);
+ }
Repeat the guard for roles/distributionQueue/distributionVerifier to prevent foot‑guns.
📝 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.
let sponsorshipQueueAddress = ethers.ZeroAddress; | |
if (network.name == "sepolia") { | |
sponsorshipQueueAddress = deployed_addresses["SponsorshipQueueModule#SponsorshipQueue"]; | |
} else if (network.name == "mainnet") { | |
// TODO: ./ignition/deployments/chain-1/deployed_addresses.json | |
} | |
console.log("sponsorshipQueueAddress:", sponsorshipQueueAddress); | |
let sponsorshipQueueAddress = ethers.ZeroAddress; | |
if (network.name === "sepolia") { | |
sponsorshipQueueAddress = deployed_addresses["SponsorshipQueueModule#SponsorshipQueue"]; | |
} else if (network.name === "mainnet") { | |
// TODO: ./ignition/deployments/chain-1/deployed_addresses.json | |
} | |
if (sponsorshipQueueAddress === ethers.ZeroAddress) { | |
throw new Error(`Missing SponsorshipQueue address for network ${network.name}`); | |
} | |
console.log("sponsorshipQueueAddress:", sponsorshipQueueAddress); |
🤖 Prompt for AI Agents
In backend/ignition/modules/QueueHandler.ts around lines 18-25, the code
defaults sponsorshipQueueAddress to ethers.ZeroAddress for unsupported networks
which lets the app continue with invalid 0x0 addresses; change this to fail-fast
or require explicit configuration: detect unsupported network names (e.g.,
hardhat/localhost) and throw a clear error or read the address from a passed-in
config/env var instead of using ZeroAddress; apply the same guard and behavior
for roles, distributionQueue, and distributionVerifier variables so none of
those fall back to ethers.ZeroAddress silently.
const queueHandler = m.contract("QueueHandler", [ | ||
rolesAddress, | ||
sponsorshipQueueAddress, | ||
distributionQueueAddress, | ||
distributionVerifierAddress | ||
]); |
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.
🛠️ Refactor suggestion
Consider parameterizing addresses for flexibility.
Expose addresses via Ignition parameters so CI/ops can inject per‑env values without code edits.
-const queueHandler = m.contract("QueueHandler", [
- rolesAddress,
- sponsorshipQueueAddress,
- distributionQueueAddress,
- distributionVerifierAddress
-]);
+const rolesParam = m.getParameter("rolesAddress", rolesAddress);
+const sponsorQParam = m.getParameter("sponsorshipQueueAddress", sponsorshipQueueAddress);
+const distQParam = m.getParameter("distributionQueueAddress", distributionQueueAddress);
+const distVParam = m.getParameter("distributionVerifierAddress", distributionVerifierAddress);
+const queueHandler = m.contract("QueueHandler", [rolesParam, sponsorQParam, distQParam, distVParam]);
📝 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.
const queueHandler = m.contract("QueueHandler", [ | |
rolesAddress, | |
sponsorshipQueueAddress, | |
distributionQueueAddress, | |
distributionVerifierAddress | |
]); | |
const rolesParam = m.getParameter("rolesAddress", rolesAddress); | |
const sponsorQParam = m.getParameter("sponsorshipQueueAddress", sponsorshipQueueAddress); | |
const distQParam = m.getParameter("distributionQueueAddress", distributionQueueAddress); | |
const distVParam = m.getParameter("distributionVerifierAddress", distributionVerifierAddress); | |
const queueHandler = m.contract("QueueHandler", [rolesParam, sponsorQParam, distQParam, distVParam]); |
🤖 Prompt for AI Agents
In backend/ignition/modules/QueueHandler.ts around lines 42 to 47, the hardcoded
addresses passed into m.contract should be parameterized so CI/ops can inject
environment-specific values; update the module to read rolesAddress,
sponsorshipQueueAddress, distributionQueueAddress, and
distributionVerifierAddress from Ignition parameters (with sensible defaults or
validations), use those parameter values when calling m.contract, and document
the required parameter names so deployment pipelines can supply them.
const sponsorshipQueue = await SponsorshipQueue.deploy(estimatedCost); | ||
|
||
return { sponsorshipQueue, languages, account1, account2 }; | ||
return { sponsorshipQueue, account1, account2 }; |
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.
💡 Verification agent
🧩 Analysis chain
Constructor simplification LGTM; purge any lingering language‑code tests/usages.
This streamlines the UX in line with elimu.ai’s mission to make sponsorships frictionless for children’s learning📖✍🏽🔢.
🏁 Script executed:
#!/bin/bash
# Ensure no references to DummyLanguages or language codes remain
rg -n 'DummyLanguages|addSupportedLanguage|languageCode|Languages\.'
Length of output: 4188
Constructor simplification LGTM — remove leftover language-code artifacts
LGTM for the constructor change; multiple lingering references to DummyLanguages / languageCode must be removed or updated:
- backend/test/QueueHandler.ts (deploys DummyLanguages; addSupportedLanguage).
- backend/test/DistributionQueue.ts (deploys DummyLanguages; addSupportedLanguage).
- backend/contracts/dummy/DummyLanguages.sol.
- backend/contracts/DistributionQueue.sol (imports ILanguages; languageCode checks).
- frontend/src/pages/distributions/add.tsx and frontend/src/components/SponsorshipSummary.tsx (languageCode usage; SimulateContractButton/WriteContractButton).
- backend/ignition/deployments/.../artifacts/* (ABI fields named "languageCode").
- backend/hardhat-gas-report.md (mentions DummyLanguages / addSupportedLanguage).
Remove or update these tests, dummy contract, frontend usages, and deployment artifacts to complete the purge.
elimu.ai's mission is to build innovative learning software that empowers out-of-school children to teach themselves basic reading📖, writing✍🏽 and math🔢 within 6 months.
export function ReadEstimatedCost() { | ||
console.debug("ReadEstimatedCost") | ||
|
||
return ( | ||
<> | ||
<select | ||
onChange={handleLanguageCodeChange} | ||
className="p-4 text-2xl text-indigo-200 bg-indigo-800 rounded-lg"> | ||
<option value="">-- Select language --</option> | ||
<option value="ENG">ENG</option> | ||
<option value="HIN">HIN</option> | ||
<option value="TGL">TGL</option> | ||
<option value="THA">THA</option> | ||
<option value="VIE">VIE</option> | ||
</select><br /> | ||
|
||
{(languageCode.length != 3) ? ( | ||
<button disabled={true} className="mt-4 p-8 text-2xl text-zinc-400 bg-zinc-300 rounded-lg"> | ||
Send 0.0001 ETH ⟠ | ||
</button> | ||
) : ( | ||
<SimulateContractButton languageCode={languageCode} /> | ||
)} | ||
<SimulateContractButton estimatedCost={0.0001} /> | ||
</> | ||
) | ||
} |
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.
🛠️ Refactor suggestion
Bug: WriteContractButton doesn’t receive estimatedCost; plus avoid float→wei pitfalls
- WriteContractButton is rendered without the estimatedCost prop, so parseEther(undefined) will throw at runtime.
- Passing 0.0001 as a number risks floating‑point rounding. Prefer strings.
Apply this diff:
export function ReadEstimatedCost() {
console.debug("ReadEstimatedCost")
return (
<>
- <SimulateContractButton estimatedCost={0.0001} />
+ <SimulateContractButton estimatedCost={"0.0001"} />
</>
)
}
-export function SimulateContractButton({ estimatedCost }: any) {
+export function SimulateContractButton({ estimatedCost }: { estimatedCost: string }) {
console.debug("SimulateContractButton");
const deploymentAddress: Address = deployed_addresses["SponsorshipQueueModule#SponsorshipQueue"] as `0x${string}`;
console.debug("deploymentAddress:", deploymentAddress);
const { isPending, isError, error, isSuccess } = useSimulateContract({
abi,
address: deploymentAddress,
functionName: "addSponsorship",
- value: parseEther(estimatedCost)
+ value: parseEther(estimatedCost)
})
...
- return <WriteContractButton />
+ return <WriteContractButton estimatedCost={estimatedCost} />
}
-export function WriteContractButton({ estimatedCost }: any) {
+export function WriteContractButton({ estimatedCost }: { estimatedCost: string }) {
console.debug("WriteContractButton");
...
onClick={() =>
writeContract({
abi,
address: deploymentAddress,
functionName: "addSponsorship",
- value: parseEther(estimatedCost)
+ value: parseEther(estimatedCost)
})
}
>
- Send {estimatedCost} ETH ⟠
+ Send {estimatedCost} ETH ⟠
Also applies to: 71-99, 101-123
🤖 Prompt for AI Agents
In frontend/src/pages/sponsorships/add.tsx around lines 61-69 (and also apply
same fix to blocks at 71-99 and 101-123), the simulated/read component is
rendering a WriteContractButton/SimulateContractButton without passing
estimatedCost (causing parseEther(undefined) runtime errors) and is passing the
amount as a JS number (risking floating-point rounding). Fix by passing the
estimatedCost prop to the button (e.g., estimatedCost={"0.0001"}) and use a
string literal for the ether value instead of a number; repeat the same change
for the other two occurrences so every
WriteContractButton/SimulateContractButton receives a string estimatedCost prop.
Summary by CodeRabbit
New Features
Performance
Documentation
Tests