Skip to content

chore: fix all solhint issues#49

Merged
dapplion merged 3 commits intognosischain:withdrawals-pullfrom
filoozom:chore/fix-import-statements
Jul 4, 2023
Merged

chore: fix all solhint issues#49
dapplion merged 3 commits intognosischain:withdrawals-pullfrom
filoozom:chore/fix-import-statements

Conversation

@filoozom
Copy link
Copy Markdown
Contributor

Uses named imports instead of global ones. Also fixes one tiny linting issue.

@filoozom filoozom force-pushed the chore/fix-import-statements branch 2 times, most recently from 8f47a23 to c52f0d9 Compare June 28, 2023 17:23
@filoozom filoozom force-pushed the chore/fix-import-statements branch from c52f0d9 to 2329b39 Compare June 28, 2023 17:25
*/
function executeSystemWithdrawals(
uint256 _deprecatedUnused,
uint256 /* _deprecatedUnused */,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unrelated diff, please revert lines 267 and 272

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, maybe I should rename the PR to "fix all solhint issues", which this does. Or do you still prefer reverting that?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh if solhint requires that change then all good 👍

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does:

,Warning: Unused function parameter. Remove or comment out the variable name to silence this warning.
   --> project:/contracts/SBCDepositContract.sol:273:9:
    |
273 |         uint256 _deprecatedUnused,
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^

@dapplion dapplion changed the title chore: fix import statements chore: fix all solhint issues Jun 28, 2023
* NOTE: This function signature is hardcoded in the Gnosis execution layer clients. Changing this signature without updating the
* clients will cause block verification of any post-shangai block to fail. The function signature cannonical spec is here
* https://github.com/gnosischain/specs/blob/master/execution/withdrawals.md
* @param _deprecatedUnused Previously `maxFailedWithdrawalsToProcess` currently deprecated and ignored
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does solhint require to remove this line? If no, please keep. If yes, then convert to non param line we need the context of that that first variable refers to

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does:

DocstringParsingError: Documented parameter "_deprecatedUnused" not found in the parameter list of the function.
   --> project:/contracts/SBCDepositContract.sol:258:5:
    |
258 |     /**
    |     ^ (Relevant source part starts here and spans across multiple lines).

I'll convert it to non-@param.

@filoozom filoozom force-pushed the chore/fix-import-statements branch from c7800b3 to e1eb471 Compare June 28, 2023 19:48
Copy link
Copy Markdown

@4rgon4ut 4rgon4ut left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good job!

@dapplion dapplion merged commit 2f7e468 into gnosischain:withdrawals-pull Jul 4, 2023
dapplion added a commit that referenced this pull request Jul 11, 2023
* Withdrawals pull

* Update maxFailedWithdrawalsToProcess variable name

* Delete settings.json

* Place all variables at the top

* Update executeSystemWithdrawals comment

* chore: fix all solhint issues (#49)

* chore: remove global imports

* chore: ignore _deprecatedUnused

* chore: re-add _deprecatedUnused comment

* chore: fix system tx inputs (#50)

* chore: add system tx with right signature

* fix: make signature public and fix tests

* Update SBCDepositContract comment

* Update SBCDepositContract.sol

---------

Co-authored-by: Lion - dapplion <35266934+dapplion@users.noreply.github.com>

---------

Co-authored-by: Philippe Schommers <philippe@schommers.be>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants