Large grant contract

[damianmarti]

First LargeGrant contract draft:

• It allow to add grants with milestones.
• Add new stage with milestones to an existing grant.
• Complete a milestone and send the funds.
• Add/remove owners implemented the same way as the current Stream contract.

Added a Test ERC20 token to be able to test it locally.

Added some tests to the LargeGrant contract.

Features that maybe we should add later:

• Change milestones in an existing stage.
• Change milestone amount.

closes #26

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
ens-pg	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Mar 19, 2025 5:38pm

[Pabl0cks]

GOOD stuff Damu!! ♥ Loved how handy is to test this with /debug and your dummy ERC token.

A few comments, could be tackled in new PRs, or ignored in other cases:

• We should send the Milestone Proof of Completion when we complete a grant, so we can add it to the CompleteMilestone event we generate, like we do with Streams
• Batch milestones completion (I think this is the first thing they would ask haha)
• Is it worth to have a Pause feature in the contract? Or is "too safe" to need to implement it

Would love a more technical check from either @technophile-04 @rin-st or @carletex

---

In case you get weird nonce errors like I did, I think it's because of the current wagmi/viem version doesn't run simulation (See scaffold-eth/scaffold-eth-2#1038 scaffold-eth/scaffold-eth-2#1049).
Maybe in the future we could think on upgrading wagmi/viem version of this project

[technophile-04]

This is looking clean and nice @damianmarti thanks!! Just added a couple of question in comment but looks good.

yeah we need the update feature for sure we can do it in other PR as well if you feel it 🙌

---

Question to @Pabl0cks :

So users will be applying for new stage like this right? (not exactly same UI) but each milestone will have amount right?

And later when they completely the milestone they go to grant detail page and beside each milestone under a stage they will complete button?

Once clicking it, this will go to admin something like and admin will see:

So as I understand Milestone struct must also have:

	struct Milestone {
		uint8 number;
		uint256 amount;
		bool completed;
                string description; // new field
                string proof; // new field
      }

So we should add those fields on chain too right?

[Pabl0cks]

Question to @Pabl0cks :

Yeah! The workflow will be like this, not sure where exactly we'll need to store these fields, but I think they need to be emitted with CompleteMilestone event so we can track them with Ponder?

[damianmarti]

Question to @Pabl0cks :

Yeah! The workflow will be like this, not sure where exactly we'll need to store these fields, but I think they need to be emitted with CompleteMilestone event so we can track them with Ponder?

The milestone description and proof are only on the web2 db now. I can add these to the onchain data if you think it's needed there.

[damianmarti]

GOOD stuff Damu!! ♥ Loved how handy is to test this with /debug and your dummy ERC token.

:-)

A few comments, could be tackled in new PRs, or ignored in other cases:

• We should send the Milestone Proof of Completion when we complete a grant, so we can add it to the CompleteMilestone event we generate, like we do with Streams

I didn't see this in the Stream contract. When are you sending this proof there?

Anyway, I can add the proof to the milestones if we need this information onchain.

• Batch milestones completion (I think this is the first thing they would ask haha)

Sure, I can add this!

• Is it worth to have a Pause feature in the contract? Or is "too safe" to need to implement it

maybe we can implement it as a security measure. Only the owners can interact with the contract, but if there is a vulnerability in some of these functions in the future, the pause feature may help to reduce the damage.

Would love a more technical check from either @technophile-04 @rin-st or @carletex

In case you get weird nonce errors like I did, I think it's because of the current wagmi/viem version doesn't run simulation (See scaffold-eth/scaffold-eth-2#1038 scaffold-eth/scaffold-eth-2#1049). Maybe in the future we could think on upgrading wagmi/viem version of this project

[rin-st]

Gj Damu! Everything works as expected!

Added question regarding requirement of the stage creation and one naming nit

[damianmarti]

Improvements:

• Better error handling on addGrant and addGrantStage.
• Admin role to add and remove owners.
• Allow admin to pause contract.
• Batch complete milestones.
• Added tests to new features.

About the milestone description and proof of completion. This data is now saved on the web2 db. We can store this data in the contract too if it's needed. Another option is to keep the data off-chain, but receiving the proof on completeMilestone and add this data to the CompleteMilestone event (not saving the data on-chain).

We can ask ENS about this, or we can choose what we think is the best option.

[technophile-04]

Another option is to keep the data off-chain, but receiving the proof on completeMilestone and add this data to the CompleteMilestone event (not saving the data on-chain).

I think this makes sense? Like emitting the event? Also is it better if we emit the milestones reason in addGrant and addStage?

[damianmarti]

Fixes and improvements:

• Removes batch complete milestones.
• Added description and proof to completeMilestone event.
• Removed AddOwner and RemoveOwner events.
• Added milestones approval
• Allow admin to change builder address

[technophile-04]

Added milestones approval

Umm I think we haven't implemented multiple approval yet right? This will allow owner / stweard to approve the grant?

[damianmarti]

Yes, one owner/steward needs to approve the milestone and then another one can complete the milestone, sending the funds.

The grants are approved off-chain before one owner/steward can create the grant on-chain.

[Pabl0cks]

GJ Damu! 🔥 Is looking good to me.

Nit: maybe we should rename role names (future PR?) to be more clear?
OWNER_ROLE => STEWARD_ROLE ?
DEFAULT_ADMIN_ROLE => CONTRACT_ADMIN_ROLE?

[damianmarti]

GJ Damu! 🔥 Is looking good to me.

Nit: maybe we should rename role names (future PR?) to be more clear? OWNER_ROLE => STEWARD_ROLE ? DEFAULT_ADMIN_ROLE => CONTRACT_ADMIN_ROLE?

Thanks @Pabl0cks for the review!

I changed the owner role to steward role but I kept the default admin role. I don't see too much upside in changing this. We can change it later if we want.