ChargedParticlesEth
ChargedParticlesEth copied to clipboard
robsecord
Bounty: Charged Particles - Unit-Tests for Solidity Contracts
The Solidity Contracts for Charged Particles need Unit-Tests! All Contracts under the ./contracts folder (except DAI.sol)
This bounty is paying 40 DAI for every 10% increase in coverage! Plus a 100 DAI bonus if the coverage reaches 100%!
That's a possible total of 500 DAI!
Requirements:
- Fork the repo and create a new branch with your username,
- Make note of the current coverage percentage before you start,
- commit changes to your branch,
- Make note of the coverage percentage after you have completed,
- Submit a Pull-Request of your branch for this issue with your submission, noting the starting and ending coverage percentage.
We're up to ~20% coverage! 420 DAI remaining to be claimed!
Issue Status: 1. Open 2. Started 3. Submitted 4. Done
This issue now has a funding of 120.0 DAI (120.0 USD @ $1.0/DAI) attached to it as part of the Charged Particles fund.
- If you would like to work on this issue you can 'start work' on the Gitcoin Issue Details page.
- Want to chip in? Add your own contribution here.
- Questions? Checkout Gitcoin Help or the Gitcoin Slack
- $69,461.29 more funded OSS Work available on the Gitcoin Issue Explorer
@robsecord I just realized that the current ChargedParticlesERC1155 contract on master does not implement IChargedParticlesERC1155, is that intended?
EDIT: Same question for ChargedParticlesEscrow and IChargedParticlesEscrow.
Ya, I used the interfaces for something else. Forget what now. I guess I could inherit from them, but it's not necessary. Good catch though, thanks!
Update: In the ChargedParticles contract, the majority of the Admin/DAO functions and all of the public read functions for an ion token have unit tests with then, right now I'm trying to create unit tests for minting ion tokens. Later on I will add more for minting a new particle or plasma.
Thanks for the update! Let me know if you have any questions or need anything from me!
I realize that there is a lot of work involved, so I have updated the original issue post.
Feel free to commit in stages, and I will payout according to the coverage achieved, but please let me know if you do not plan to continue so that I or others can pick up where you left off.
@robsecord I noticed one thing -- the withdrawFees function does not set the collectedFees for that particular _contractOwner back to 0. Is this intended, or is it a bug?
@robsecord I'm still going to write some more unit tests, the last one there was just partially done so that you may be unblocked on other tasks that you may want to use them on.
Thanks for the update! I have made some slight changes to the file structure. Please pull latest. I have also tried to get coverage working to no avail.. It seems it just doesn't want to work yet.. I have also committed a fix for the withdraw functions that you mentioned. Thanks again for finding that!
@robsecord Sorry, the withdrawFees function in ChargedParticles.sol is still not resetting the associated collectedFees back to 0. In the unit tests, it is still impossible to repeatedly withdraw fees indefinitely, because it is checked by Address.sol when you call sendValue, but this doesn't sound like a proper solution to the problem.
Ahh, I fixed the withdraw functions in the escrow! Didn't realize you meant the one in the controller.. I have now fixed that too, pull latest! Thx again!
Any update on this? If not it's ok, I will pay out a portion of the amount for the work done so far.
@robsecord Hey, sorry I dropped the ball on this one. I had to deal with some personal issues and this slipped my mind. I have a small enhancement on the unit test that I have locally, will make a PR shortly. After that, you may proceed with the pay out.
Hey, no problem, thx for the work so far, and let me know if you want to pick the ball back up! I want to keep the bounty open in order to get it completed, so I will send you a tip on Gitcoin for the work done. I first need to review what you have just submitted and see if I can get a coverage report working so I can get the numbers. I will keep you posted, should be able to check this out tomorrow evening (eastern timezone).
Hey @robsecord, thanks for your patience! Oh, and also for your tip!
Somehow, your comment escaped my notifications, and I saw it only now.
Yes, I started the task, however, I found little to no time to complete it, I've been working mostly to complete my other started bounty, but now that that's done, I can focus on this.
I saw that the project has a remarkable combination of truffle, buidler, jest, openzeppelin, all mashed up together in one big cauldron. Now, from my knowledge, jest can't measure coverage with solidity contracts (neither with truffle, neither with buidler) so, there is no way to measure coverage in the current cauldron configuration (but please correct me if I'm wrong).
Anyway, I'm on it starting now, will keep you updated.
You're right, I have had a nightmare trying to make Jest + Coverage work.. We can drop Jest and stick with Buidler + Mocha. And I plan to move away from Truffle and go entirely with Buidler (might even switch to ethers.js too), so feel free to tackle this any way you want!
⚡️ A tip worth 40.00000 DAI (40.0 USD @ $1.0/DAI) has been granted to @kichjang for this issue from @robsecord. ⚡️
Nice work @kichjang! Your tip has automatically been deposited in the ETH address we have on file.
- $120178.40 in Funded OSS Work Available at: https://gitcoin.co/explorer
- Incentivize contributions to your repo: Send a Tip or Fund a PR
- No Email? Get help on the Gitcoin Chat
Hey @robsecord, sorry for this, but I'll have to stop work. This just evades my time and it's better if I let it open to other people. Hopefully, I'll restart work sometime in the future, but thanks again for your patience.
⚡️ A tip worth 5.00000 DAI (5.0 USD @ $1.0/DAI) has been granted to @uivlis for this issue from @robsecord. ⚡️
Nice work @uivlis! Your tip has automatically been deposited in the ETH address we have on file.
- $176025.46 in Funded OSS Work Available at: https://gitcoin.co/explorer
- Incentivize contributions to your repo: Send a Tip or Fund a PR
- No Email? Get help on the Gitcoin Chat
Issue Status: 1. Open 2. Started 3. Submitted 4. Done
Work has been started.
These users each claimed they can complete the work by 266 years, 4 months from now. Please review their action plans below:
1) uivlis has started work.
I just cleaned up some tests, and I may be wrong, but it seems you might have some left-over code which creates errors when minting particles, for example.
Learn more on the Gitcoin Issue Details page.
Hey @robsecord just went again through the code. You had a typo somewhere in a require (I think), and a lot of functions in escrowMgr that I don't know where and how to call in order for the minting particles test to work. Could you explain a bit that part of contract logic to me, so that I can move the testing forward? You can look at the PR to see my comments added to the functions I was talking about. Also, I upgraded the dependencies, so I had to remove some incompatible logging configuration somewhere in the deployment scripts.
⚡️ A tip worth 40.00000 DAI (40.0 USD @ $1.0/DAI) has been granted to @uivlis for this issue from @robsecord. ⚡️
Nice work @uivlis! Your tip has automatically been deposited in the ETH address we have on file.
- $213967.28 in Funded OSS Work Available at: https://gitcoin.co/explorer
- Incentivize contributions to your repo: Send a Tip or Fund a PR
- No Email? Get help on the Gitcoin Chat
@uivlis are you on Discord? would be great to chat there, and I can explain things a bit better for you. Otherwise, everything looks good, I left a couple of comments but still merged the PR. Works well, and we're up to 19% coverage so I tipped you the 40 DAI for increasing coverage to ~20%.
Charged Particles on Discord: https://discord.gg/Syh3gjz
Thanks for the tip, I just joined the server. You can DM me, or else I'll have to wait some minutes until I can post in the bounties channel.