QA: Solidity style coding convention

[petrovska-petro]

Severity: informational / best practices

Context: the code currently does not follow the standard style format at the smart contract level described in the official guide and as well seems not to have any type of prettier into the codebase, which affects readability.

Recommendation: for better readability follow the style guide link shared above and also it will be at par with industry standard on the style convention of how to organise a smart contract in Solidity.

Things like having a modifier all the way down into the file it's a bit messy and reviewers/integrators who are acustom to the official style-guide may find it unnatural to read a broken pattern of smart contract style.

Actionables:

1. Use a prettier
2. Follow the style guide convention from the official documentation

Review stage

Balancer Maxis:

• Acknowledge
• Fixed

Onchainification Labs:

• Acknowledge correction/mitigation

[Tritium-VLK]

This is a second review of this code, which is an adaptation of Injector V1 that has been running without problems for a long time. The goal here was to make some UX improvements and have an incremental change.

Over this review process it has come up that @gosuto-inzasheru thinks that we should do a bigger overhaul of the injector. Once we have a clear scope/spec and can estimate out the time it will take/etc we can do that. In this case the question is really understanding the risks presented in the code.

I can run a prettifier once we merge all the open PRs. I'd rather not make a lot of changes to style here and now unless it really makes a difference in your ability to follow/wrap your head around the code.

There is some possibility that we will want to use this code for a PoC/new economic model on MODE chain in a month. The goal here is to understand the potential risks of doing that. Note that we may in the end decide to use a different model as well and can find a way to work around injector V2 for the first weeks, even if it is perfect for purpose until we can deal we are comfy.

[petrovska-petro]

@Tritium-VLK completely understand the final goal, let's leave this issue open to simply remember to pretify the codebase once all PRs are being merged

[petrovska-petro]

Using this issue for general informative purposes, since it's only a placeholder/reminder for the prettier

@Tritium-VLK we concluded the review for commit 444ee28. Next we can proceed reviewing the PRs and ensuring that all issues are being tackled

[Tritium-VLK]

@petrovska-petro sounds good. I have gone ahead and assigned all issues to you that I think are concluded. There is still one open L issue in #33. Let me make sure that is addressed, and get everything assigned back to you. Then we can go ahead and review prs, merge, and do a final review.

I think in terms of testing, the question is, are you confident this contract won't get us rekt and the safeguards will work as expected. In the end we will probably launch this as v2.0beta to start testing and get our feet wet? Then likely rebuild it into foundry and/or brownie with more robust testing and better deployment scripts if we think we are going to be using it/deploying it a lot.

[Tritium-VLK]

ok @petrovska-petro can you please review (and feel free to merge if they look good) all the PRs, and let me know if there are any open issues that you feel are still important to address. Assign any issues that are still open back to me. Please go ahead and close anything you acknowledge as done.

I'd like to take a final look the code and ask you to do the same once all the PRs are merged. It's hard for me to have a sense of the end result at the moment.

[petrovska-petro]

@Tritium-VLK as informational note, i do not hold rights to assign you in any issue neither to merge PRs which are g2g.

could you handle the merging side of things or perhaps change my rights in this particular repo?

[Tritium-VLK]

can do. I wasn't able to merge either actually, but I think gos made you an editor/now we're in good shape yeah?

[petrovska-petro]

basic linting has being pushed in #47

[petrovska-petro]

linting was successfully merged in #47 and deployments has being handled with latest main;

release deployment-1 had being created matching latest commit ✅