Closed rndquu closed 5 months ago
I thought there was a reason why we didn't implement echidna when I asked about it
I thought there was a reason why we didn't implement echidna when I asked about it
At that time we didn't have a clear scope of what to fuzz. Now we have the LibUbiquityPool which is going to be deployed on mainnet, so the contract scope is clear.
I thought there was a reason why we didn't implement echidna when I asked about it
I think that auditors from our 1st Sherlock audit have already performed all of the fuzz, invariant and formal verification tests so it makes sense to set a normal priority for:
I thought there was a reason why we didn't implement echidna when I asked about it
I think that auditors from our 1st Sherlock audit have already performed all of the fuzz, invariant and formal verification tests so it makes sense to set a normal priority for:
Good point, I remember that quite a few issues were detected with different fuzzers on top of manual code reviews.
@gitcoindev the deadline is at 2024-05-09T10:02:25.473Z
/start
! Too many assigned issues, you have reached your max limit
! Too many assigned issues, you have reached your max limit
I will work on this behind the scenes and re-start the bot when the other issues are closed.
@gitcoindev do you mind if i work on this one,if you have other tasks?
@gitcoindev do you mind if i work on this one,if you have other tasks?
Hi @3scava1i3r I have already started (the other tasks are finished, just waiting to be officially closed), but there is good news for you. We have a twin task for invariant tests here: https://github.com/ubiquity/ubiquity-dollar/issues/563 , would you mind picking that one instead?
@3scava1i3r please confirm, whether you would be fine working on the twin task mentioned in the comment above - invariant test. If you are willing to work on this, please execute start command in https://github.com/ubiquity/ubiquity-dollar/issues/563 and the task will be automatically assigned to you.
Hey there! @gitcoindev 👋 I've been working on the 'Better Security' tasks and wanted to ask for some assistance. I tried using the 'start' command to pick a task, but it doesn't seem to be working for me.
I've made some progress and don't want to interrupt the workflow or cause any issues. Could you please guide me on how to pick a task properly, so we can ensure a smooth collaboration? Thank you so much for your time and help! 🙏 Really excited to contribute to this project!
@3scava1i3r please confirm, whether you would be fine working on the twin task mentioned in the comment above - invariant test. If you are willing to work on this, please execute start command in https://github.com/ubiquity/ubiquity-dollar/issues/563 and the task will be automatically assigned to you.
No sir,I will find something else to work on then
Hi @0xJoichiro . Let's try to fix picking the task for you. There are a few commands available in the workflow. If you type /help
command like I will do below, all the commands will be listed. In order to start a task, you just need to type /start
in comment. Please let me know which tasks you have problem with and let's try to assign this to you. After you execute /start
command you should open a draft pull request within a time frame mentioned in the issue label. The draft pull request does not need to have any major changes, can be just a comment. If you push code to the pull request branch and on a daily basis add comments to the issue you are working on, the bot will keep the task assigned to you until you solve it.
/help
Command | Description | Example |
---|---|---|
/start |
Assign yourself to the issue. | /start |
/stop |
Unassign yourself from the issue. | /stop |
/help |
List all available commands. | /help |
/query |
Returns the user's wallet, access, and multiplier information. | /query @user |
/ask |
Ask a context aware question. | /ask is x or y the best approach? |
/multiplier |
Set the task payout multiplier for a specific contributor, and provide a reason for why. | /multiplier @user 0.5 "multiplier reason" |
/labels |
Set access control, for admins only. | /labels @user priority time price |
/authorize |
Approve a label change, for admins only. | /authorize |
/wallet |
Register your wallet address for payments. | /wallet ubq.eth |
/help
# Skipping to list available commands.
Hi @0xJoichiro . Let's try to fix picking the task for you. There are a few commands available in the workflow. If you type
/help
command like I will do below, all the commands will be listed. In order to start a task, you just need to type/start
in comment. Please let me know which tasks you have problem with and let's try to assign this to you. After you execute/start
command you should open a draft pull request within a time frame mentioned in the issue label. The draft pull request does not need to have any major changes, can be just a comment. If you push code to the pull request branch and on a daily basis add comments to the issue you are working on, the bot will keep the task assigned to you until you solve it.
understood currently working on https://github.com/ubiquity/ubiquity-dollar/issues/927 using openzeplin defender
fuzzing testing upcoming? @gitcoindev
fuzzing testing upcoming? @gitcoindev
Yes, I will open a pull request until Friday EOD.
I (no AI involved) identified the following properties of the system in case of mint, redeem and collection redemption :
Dollar token mint properties:
Dollar slippage
returned.Collateral slippage
returned.Governance slippage
returned.Dollar token redeem properties:
Dollar price too high
.Collateral slippage
is returned and Dollars cannot be redeemed.Governance slippage
is returned and Dollars cannot be redeemed.Collecting redemption properties:
Those properties will be a base for the fuzz tests. Multiple inputs shall be provided by the test harness to test each of the properties.
I (no AI involved) identified the following properties of the system in case of mint, redeem and collection redemption :
Dollar token mint properties:
- Given proper amount of governance tokens and collateral tokens for non 1-to-1 mint, when Dollar price is too low for the mint price threshold, no Dollar tokens can be minted. The same applies for 1-to-1 mint with governance tokens only.
- If minimal amount of Dollars requested to be minted is higher than amount that can be minted, including the subtracted minting fee, no Dollar tokens can be minted with
Dollar slippage
returned.- If provided amount of collateral is not enough to cover the mint, no Dollar tokens can be minted with
Collateral slippage
returned.- If provided amount of governance tokens is not enough to cover the mint for 1-to-1 and fractional mint, no Dollar tokens can be minted with
Governance slippage
returned.- The Dollars can be minted for collateral ratio to governance tokens between 0 to 100. This means that it should be possible to mint Dollar tokens just providing collateral tokens, just providing governance tokens or the combination of both that will satisfy total collateral needed to mint given amount of tokens.
Dollar token redeem properties:
- Dollar cannot be redeemed if the the dollar price is above the pool threshold, returning
Dollar price too high
.- There must be sufficient amount of collateral in the pool to redeem dollars.
- If amount of collateral requested to redeem is higher then calculated based on current price,
Collateral slippage
is returned and Dollars cannot be redeemed.- If amount of governance tokens requested to redeem is higher then calculated based on current price,
Governance slippage
is returned and Dollars cannot be redeemed.- The Dollars can be redeemed for collateral ratio to governance tokens between 0 to 100. This means that it should be possible to redeem just collateral tokens, just governance tokens or the combination of both that will satisfy total collateral needed to redeem given amount of Dollar tokens.
Collecting redemption properties:
- Given any amount of governance tokens and collateral token balances that are available to be collected after the redemption, they can be collected after certain number of blockchain blocks pass since the last redemption happened for the same account.
Those properties will be a base for the fuzz tests. Multiple inputs shall be provided by the test harness to test each of the properties.
Sounds good. The same "properties" could be later converted to invariants.
/start
Warning! | This task was created over 38 days ago. Please confirm that this issue specification is accurate before starting. |
Deadline | Sat, May 25, 2:21 PM UTC |
Registered Wallet | 0x7e92476D69Ff1377a8b45176b1829C4A5566653a |
/wallet 0x0000...0000
if you want to update your registered payment wallet address.Draft pull request with a skeleton for tests opened. I will mark it as ready for review when all the fuzz tests based on properties above will be implemented.
Pull request ready for the review.
/start
! Too many assigned issues, you have reached your max limit
! Too many assigned issues, you have reached your max limit
Interesting, I received a notification in the pull request that bot unassigned me, so I tried to re-assign myself.
+ Evaluating results. Please wait...
View | Contribution | Count | Reward |
---|---|---|---|
Issue | Comment | 1 | 1.7 |
Comment | Formatting | Relevance | Reward |
---|---|---|---|
I thought there was a reason why we didn't implement echidna whe... | 1.7 | 0.56 | 1.7 |
View | Contribution | Count | Reward |
---|---|---|---|
Issue | Comment | 1 | 0.4 |
Review | Comment | 1 | 7.7 |
Comment | Formatting | Relevance | Reward |
---|---|---|---|
fuzzing testing upcoming? @gitcoindev ... | 0.4 | 0.865 | 0.4 |
hi @gitcoindev can we instead organize the tests inside a "fuzz"... | 7.7code: count: 1 score: "1" words: 10 | 0.725 | 7.7 |
View | Contribution | Count | Reward |
---|---|---|---|
Issue | Task | 1 | 300 |
Issue | Comment | 10 | 0 |
Issue | Comment | 10 | 102.8 |
Review | Comment | 7 | 29.8 |
Review | Comment | 7 | 29.8 |
View | Contribution | Count | Reward |
---|---|---|---|
Issue | Specification | 1 | 46 |
Issue | Comment | 3 | 41.8 |
Review | Comment | 2 | 28 |
Comment | Formatting | Relevance | Reward |
---|---|---|---|
We should implement fuzzing tests for [LibUbiquityPool](https://... | 46a: count: 4 score: "4" words: 9 li: count: 8 score: "8" words: 114 code: count: 2 score: "2" words: 5 | 1 | 46 |
> I thought there was a reason why we didn't implement echidn... | 7.6a: count: 1 score: "1" words: 1 | 0.85 | 7.6 |
> I thought there was a reason why we didn't implement echidn... | 14li: count: 3 score: "3" words: 24 | 0.86 | 14 |
> I (no AI involved) identified the following properties of t... | 20.2a: count: 1 score: "1" words: 1 li: count: 11 score: "11" words: 359 code: count: 6 score: "6" words: 14 | 0.815 | 20.2 |
> I am playing with the fuzzer a lot and discovered an intere... | 12.2code: count: 3 score: "6" words: 5 | 0.81 | 12.2 |
@gitcoindev 1. Could you add 2 more fuzz tests: a) If user ... | 15.8li: count: 2 score: "4" words: 11 | 0.705 | 15.8 |
View | Contribution | Count | Reward |
---|---|---|---|
Issue | Comment | 2 | 11.35 |
Comment | Formatting | Relevance | Reward |
---|---|---|---|
Hey there! @gitcoindev 👋 I've been working on the 'Better Secu... | 9.1 | 0.8 | 9.1 |
> Hi @0xJoichiro . Let's try to fix picking the task for you.... | 2.25code: count: 3 score: "0.75" words: 3 | 0.885 | 2.25 |
View | Contribution | Count | Reward |
---|---|---|---|
Issue | Comment | 1 | 0.4 |
Review | Comment | 1 | 0.558 |
Comment | Formatting | Relevance | Reward |
---|---|---|---|
I thought there was a reason why we didn't implement echidna whe… | 1.6content: p: count: 16 score: 1 wordValue: 0.1 formattingMultiplier: 1 | 0.25 | 0.4 |
2% seems steep? Why was this decided to be the default? Please t… | 3.1content: p: count: 31 score: 1 wordValue: 0.1 formattingMultiplier: 1 | 0.18 | 0.558 |
View | Contribution | Count | Reward |
---|---|---|---|
Issue | Specification | 1 | 16.2 |
Issue | Comment | 3 | 10.273 |
Review | Comment | 4 | 11.0655 |
Comment | Formatting | Relevance | Reward |
---|---|---|---|
We should implement fuzzing tests for [LibUbiquityPool](https://… | 16.2content: p: count: 151 score: 1 a: count: 9 score: 1 code: count: 2 score: 1 wordValue: 0.1 formattingMultiplier: 1 | 1 | 16.2 |
At that time we didn't have a clear scope of what to fuzz. Now w… | 6.6content: p: count: 32 score: 1 a: count: 1 score: 1 wordValue: 0.2 formattingMultiplier: 1 | 0.845 | 5.577 |
I think that auditors from our 1st Sherlock audit have already p… | 7.4content: p: count: 37 score: 1 wordValue: 0.2 formattingMultiplier: 1 | 0.44 | 3.256 |
Sounds good. The same "properties" could be later converted to [… | 2.4content: p: count: 11 score: 1 a: count: 1 score: 1 wordValue: 0.2 formattingMultiplier: 1 | 0.6 | 1.44 |
@0x4007 That 2% fee is used solely in the test files. Mint and … | 2.6content: p: count: 25 score: 1 a: count: 1 score: 1 wordValue: 0.1 formattingMultiplier: 1 | 0.31 | 0.806 |
@gitcoindev How did you come up with the `90000` number?… | 3.1content: p: count: 28 score: 1 code: count: 2 score: 1 a: count: 1 score: 1 wordValue: 0.1 formattingMultiplier: 1 | 0.85 | 2.635 |
This seems to be a low severity issue but keeping in mind that n… | 3.2content: p: count: 31 score: 1 code: count: 1 score: 1 wordValue: 0.1 formattingMultiplier: 1 | 0.81 | 2.592 |
@gitcoindev 1. Could you add 2 more fuzz tests: a) If user mint… | 6.1content: p: count: 61 score: 1 wordValue: 0.1 formattingMultiplier: 1 | 0.825 | 5.0325 |
View | Contribution | Count | Reward |
---|---|---|---|
Issue | Task | 1 | 300 |
Issue | Comment | 10 | 0 |
Review | Comment | 10 | 90.696 |
Comment | Formatting | Relevance | Reward |
---|---|---|---|
Good point, I remember that quite a few issues were detected wit… | 0content: p: count: 20 score: 1 wordValue: 0 formattingMultiplier: 0 | 0.57 | - |
I will work on this behind the scenes and re-start the bot when … | 0content: p: count: 18 score: 1 wordValue: 0 formattingMultiplier: 0 | 0.36 | - |
Hi @3scava1i3r I have already started (the other tasks are finis… | 0content: p: count: 42 score: 1 wordValue: 0 formattingMultiplier: 0 | 0.35 | - |
@3scava1i3r please confirm, whether you would be fine working on… | 0content: p: count: 44 score: 1 wordValue: 0 formattingMultiplier: 0 | 0.22 | - |
Hi @0xJoichiro . Let's try to fix picking the task for you. Ther… | 0content: p: count: 143 score: 1 code: count: 3 score: 1 wordValue: 0 formattingMultiplier: 0 | 0.68 | - |
Yes, I will open a pull request until Friday EOD. | 0content: p: count: 10 score: 1 wordValue: 0 formattingMultiplier: 0 | 0.76 | - |
I (no AI involved) identified the following properties of the sy… | 0content: p: count: 408 score: 1 code: count: 14 score: 1 wordValue: 0 formattingMultiplier: 0 | 0.725 | - |
Draft pull request with a skeleton for tests opened. I will mark… | 0content: p: count: 29 score: 1 wordValue: 0 formattingMultiplier: 0 | 0.69 | - |
Pull request ready for the review. | 0content: p: count: 6 score: 1 wordValue: 0 formattingMultiplier: 0 | 0.715 | - |
Interesting, I received a notification in the pull request that … | 0content: p: count: 19 score: 1 wordValue: 0 formattingMultiplier: 0 | 0.73 | - |
Resolves https://github.com/ubiquity/ubiquity-dollar/issues/925 … | 0content: p: count: 7 score: 1 wordValue: 0 formattingMultiplier: 0 | 0.71 | - |
@0x4007 yes, as @rndquu mentioned, on deployment fees are set to… | 8.4content: p: count: 21 score: 1 wordValue: 0.2 formattingMultiplier: 2 | 0.6 | 5.04 |
@rndquu when I executed longer fuzzing sessions I frequently enc… | 32.4content: p: count: 81 score: 1 wordValue: 0.2 formattingMultiplier: 2 | 0.67 | 21.708 |
This pull request is still a draft. It contains a skeleton for f… | 12content: p: count: 30 score: 1 wordValue: 0.2 formattingMultiplier: 2 | 0.62 | 7.44 |
I am playing with the fuzzer a lot and discovered an interesting… | 42content: p: count: 92 score: 1 code: count: 13 score: 1 wordValue: 0.2 formattingMultiplier: 2 | 0.65 | 27.3 |
I will finish with the whole test suite till Friday EOD and set … | 7.6content: p: count: 19 score: 1 wordValue: 0.2 formattingMultiplier: 2 | 0.59 | 4.484 |
I pushed the remaining fuzz tests also for redemption delay, set… | 6.4content: p: count: 16 score: 1 wordValue: 0.2 formattingMultiplier: 2 | 0.625 | 4 |
Sure, thank you for the feedback. | 2.4content: p: count: 6 score: 1 wordValue: 0.2 formattingMultiplier: 2 | 0.66 | 1.584 |
Hi @rndquu @0x4007 @molecula451 the PR is ready to review, I exe… | 22.8content: p: count: 57 score: 1 wordValue: 0.2 formattingMultiplier: 2 | 0.65 | 14.82 |
@molecula451 I moved fuzz tests as requested, could you please h… | 6.4content: p: count: 16 score: 1 wordValue: 0.2 formattingMultiplier: 2 | 0.675 | 4.32 |
View | Contribution | Count | Reward |
---|---|---|---|
Issue | Comment | 2 | 0.1225 |
Comment | Formatting | Relevance | Reward |
---|---|---|---|
@gitcoindev do you mind if i work on this one,if you have other … | 0.35content: p: count: 14 score: 1 wordValue: 0.1 formattingMultiplier: 0.25 | 0.2 | 0.07 |
No sir,I will find something else to work on then | 0.25content: p: count: 10 score: 1 wordValue: 0.1 formattingMultiplier: 0.25 | 0.21 | 0.0525 |
View | Contribution | Count | Reward |
---|---|---|---|
Issue | Comment | 2 | 0.0445 |
Comment | Formatting | Relevance | Reward |
---|---|---|---|
Hey there! @gitcoindev 👋 I've been working on the 'Better Secu… | 2.225content: p: count: 89 score: 1 wordValue: 0.1 formattingMultiplier: 0.25 | 0.02 | 0.0445 |
understood currently working on https://github.com/ubiquity/ubiq… | 0.2content: p: count: 8 score: 1 wordValue: 0.1 formattingMultiplier: 0.25 | - | - |
View | Contribution | Count | Reward |
---|
Comment | Formatting | Relevance | Reward |
---|
We should implement fuzzing tests for LibUbiquityPool. I think it would be enough to fuzz only "user scenarios":
Possible solutions for fuzzing tests (I would simply start with foundry since we're using it as a development framework + it also contains invariant testing):
What should be done:
UbiquityPoolFacet.fuzz.t.sol
so we could distinguish unit and fuzz tests)development
branch fuzzing tests should run with a great number of runs to test many cases (keep in mind that github action runnners can run for 6 hours)