Streaming Grant Mecahnics

[apbendi]

Basic functionality for issuing & revoking streaming grants via Sablier.

Aside from needing some cleanup & cleaning up the commit history I think this is good to go. I also had to increase gas limits (not sure if it's real or a bug) but I set them to super high numbers. Need to investigate this.

closes #6
closes #7
closes #11

[mds1]

I also had to increase gas limits (not sure if it's real or a bug) but I set them to super high numbers. Need to investigate this.

I had a similar issue and created #16 for it. If you have any details to add that would probably be a good place to put them

[mds1]

Some questions/thoughts:

1. We should probably rename Moloch to Endaoment everywhere
2. What was the rationale for removing Ownable.sol and implementing similar (but reduced) functionality manually?
3. If a grant or revocation proposal does not pass, I think we do not need to do anything in the else blocks where it says // TODO: anything?
4. Pool.sol: Do we need this file? It's not used. I haven't looked at it closely but I don't remember what it's used for
5. I'd recommend putting dai.json in a folder called abi at the project root, and adding a file called addresses.json at the project root with all relevant contract address. We'll need those in both the contracts and frontend so probably should have it all in one spot
   • Oh just noticed you're using environment variables for the addresses so that works too

Otherwise, all tests passed! 🥳

Other general question—in the tests I noticed you do things like this.instance in the before() hook, whereas with Truffle tests I (we?) previously used let instance; just above the before hook and instance = ... within the hook when creating the contract. Is there any tradeoffs / reasons / considerations here as to the differences in the approaches?

[apbendi]

Great feedback. Thanks!

We should probably rename Moloch to Endaoment everywhere

Good call. Done.

What was the rationale for removing Ownable.sol and implementing similar (but reduced) functionality manually?

Good question. So the new OZ contracts all use the "initializer" pattern, which broke Moloch's assumptions around the owner being set automatically in the constructor. When I realized this, I tried calling the initialize function but then I got out of gas errors. Didn't seem worth fighting with especially because the Endaoment contract owns the Guildbank and literally can't utilize the rest of OZ's Ownable implementation (unless we expanded it).

If a grant or revocation proposal does not pass, I think we do not need to do anything in the else blocks where it says // TODO: anything?

Don't think so either. Removed.

Pool.sol: Do we need this file? It's not used. I haven't looked at it closely but I don't remember what it's used for

Agreed don't think we need it.

I'd recommend putting dai.json in a folder called abi at the project root, and adding a file called addresses.json at the project root with all relevant contract address. We'll need those in both the contracts and frontend so probably should have it all in one spot

Ahh yes good call, will do this.

Other general question—in the tests I noticed you do things like this.instance in the before() hook, whereas with Truffle tests I (we?) previously used let instance; just above the before hook and instance = ... within the hook when creating the contract. Is there any tradeoffs / reasons / considerations here as to the differences in the approaches?

No major tradeoffs I'm aware of. In fact if my understand of JavaScript is correct they're functionally the same. Not sure why I used this this time 🤷‍♂️

[apbendi]

OK! I've made changes for all your suggestions and rebased to your (now merged) branch. If it looks good to you, feel free to merge!