fix: solidity warnings

[molecula451]

Resolves #692

Original requirements by @rndquu

Right now we have some compiler warnings

What should be done:

1. Fix compiler warnings

This PR fixes all compiler warnings!

fix.mp4

2. Make sure build and test workflow fails on any new warning introduced

This PR Indeed! makes the workflow 'Build and Test' fail when any new warning introduced

there is a CI build here that shows how the builds fails https://github.com/molecula451/ubiquity-dollar/actions/runs/5355653683/jobs/9714193621

But there is more. This PR comes with an update to foundry on the CI flow necessary to match the EVM version for the new update impossible without updating the builds

[ubiquibot]

Deployment: Fri Jun 23 2023 12:58:58 (UTC)
Deployment: Fri Jun 23 2023 13:22:11 (UTC)
Deployment: Fri Jun 23 2023 13:35:27 (UTC)
Deployment: Fri Jun 23 2023 13:50:22 (UTC)
Deployment: Fri Jun 23 2023 16:34:48 (UTC)
Deployment: Fri Jun 23 2023 16:56:07 (UTC)
Deployment: Sat Jun 24 2023 16:38:16 (UTC)
Deployment: Sat Jun 24 2023 16:38:17 (UTC)
Deployment: Sat Jun 24 2023 17:22:41 (UTC)
Deployment: Sun Jun 25 2023 04:24:59 (UTC)
Deployment: Sun Jun 25 2023 05:03:04 (UTC)
Deployment: Sun Jun 25 2023 05:25:11 (UTC)
Deployment: Sun Jun 25 2023 20:01:17 (UTC)
Deployment: Mon Jun 26 2023 02:50:39 (UTC)
Deployment: Mon Jun 26 2023 08:43:46 (UTC)
Deployment: Mon Jun 26 2023 08:44:35 (UTC)
Deployment: Mon Jun 26 2023 09:17:20 (UTC)
Deployment: Mon Jun 26 2023 09:17:55 (UTC)
Deployment: Mon Jun 26 2023 09:45:29 (UTC)
Deployment: Mon Jun 26 2023 09:45:30 (UTC)

[molecula451]

let's finish this one, it looks like @zgorizzo69 forgot to merge? when asked? @rndquu

[0x4007]

Still skeptical about the removal of that line

[molecula451]

Still skeptical about the removal of that line

bad coverage, coveralls.io, no testing, the whole function might need refactoring

[molecula451]

Still skeptical about the removal of that line

am keeping it at, 9e0fab5 best & approach it's to refactor and build tests cases that's another scope, there is not compiler warning, with this one either, again, best approach refactor and test cases also @rndquu said something about this one

[molecula451]

also to stay on the grind, this line wasn't doing anything after a return statement, in it's original commit, that's why I commented out + it's giving me the coveralls fails, this PR uncovers the bug within and that it needs refactor @rndquu @pavlovcik

[molecula451]

To sum up things:

1. The line was commented out because it was doing anything, and it's not doing anything as per original repo as this is unreachable code there
   

2. It'd probably get cleaned by rndquu mention

3. The PR uncovers the bug so it's another contribution

4. A refactor it's possible at the discretion of the team but not the scope of the PR

5. It's seem a simple solution the pavlovcik suggetion, but it gives the contributor (me) bad coverage, again a refactor might be need it

6. Lastly and not least the PR performs very well according to original specs as it contains a smooth clean up of compiler warnings

[0x4007]

I made a new bounty thanks for the summary of the issue @molecula451

[0x4007]

I dont think that coveralls is a great product. It alleges that coverage fell on this line, which was not changed.

Either that, or I do not understand how that product works. Please advise and merge if its a non-issue @rndquu

https://github.com/ubiquity/ubiquity-dollar/pull/704/files/0aad006a1c2d8a613d465e7e58c141285ff51b32#diff-f71aa89ff139850c01ef6582f349a23cdaac0dcc5eff20a5557cd3a5cb7a6e5bR131

[molecula451]

I dont think that coveralls is a great product. It alleges that coverage fell on this line, which was not changed.

Either that, or I do not understand how that product works. Please advise @rndquu

https://github.com/ubiquity/ubiquity-dollar/pull/704/files/0aad006a1c2d8a613d465e7e58c141285ff51b32#diff-f71aa89ff139850c01ef6582f349a23cdaac0dcc5eff20a5557cd3a5cb7a6e5bR131

Amazing thanks for the heads up, on this one.

It looks like coveralls takes in count tests made, upon research i notice this specific library doesn't have many tests case, it probably be the case why the coverage fails a bit, only on coveralls. However.

This line was after all not doing anything. I know it calls our oracle, but not after the return statement

[molecula451]

this is a clean PR, retested with all new commits, working smooth, coveralls is the only that keeps same state, also i notice after all the commits

[0x4007]

Please advise and merge if its a non-issue @rndquu

I think we should diagnose the root issue for why coveralls is failing, and possibly use a new coverage solution if we can't easily fix it. I vote that it's okay to merge this in but still waiting for rndquu's decision on it.

[molecula451]

Please advise and merge if its a non-issue @rndquu

I think we should diagnose the root issue for why coveralls is failing, and possibly use a new coverage solution if we can't easily fix it. I vote that it's okay to merge this in but still waiting for rndquu's decision on it.

rndquus, is the man, but its best to commit to avoid branch conflicts, now retesting this

[molecula451]

#687 breaks this PR as it couldn't sync the commits to this branch

[rndquu]

#687 breaks this PR as it couldn't sync the commits to this branch

Yes, this PR updates dependencies so chances are that new warnings are introduced. Pls refactor this PR again and we will merge this PR as soon as possible.

[molecula451]

#687 breaks this PR as it couldn't sync the commits to this branch

Yes, this PR updates dependencies so chances are that new warnings are introduced. Pls refactor this PR again and we will merge this PR as soon as possible.

I wish, but it didn't commit the whole branch, closing, done in #708