audit: Function Parameter Location Optimizations

[Orland0x]

closes #179

[pscott]

They mention _assertProposalExists, assertNoDuplicateIndices, bytesToAddress, _verify* functions. Any reason we don't address those?

[Orland0x]

They mention _assertProposalExists, assertNoDuplicateIndices, bytesToAddress, _verify* functions. Any reason we don't address those?

I modified `assertNoDuplicateIndices.

I changed _verifyVoteSig but couldnt do the same for _verifyProposeSig and _verifyUpdateProposalSig as I ran into stack too deep errors when using calldata, so decided to leave as memory considering those operations are not super gas critical anyway.

Not sure whether it is actually possible to use calldata in bytesToAddress, hence why the original lib i took it from didnt.

we call _assertProposalExists with a Propose storage variable in execute and cancel, so cannot prevent it being copied to memory. We could remove the function call and check proposal.startTimestamp == 0 directly but keeping the _assertProposalExists abstraction is more readable imo. thoughts?

[codecov]

Codecov Report

Merging #199 (438b4f6) into main (c73eb71) will decrease coverage by 1.04%.
The diff coverage is 55.00%.

@@            Coverage Diff             @@
##             main     #199      +/-   ##
==========================================
- Coverage   83.25%   82.22%   -1.04%     
==========================================
  Files          29       29              
  Lines         442      450       +8     
  Branches      112      114       +2     
==========================================
+ Hits          368      370       +2     
- Misses         57       64       +7     
+ Partials       17       16       -1     

Impacted Files	Coverage Δ
src/utils/SXUtils.sol	0.00% <0.00%> (ø)
src/utils/SignatureVerifier.sol	93.33% <ø> (ø)
src/Space.sol	96.18% <83.33%> (+0.79%)	⬆️
src/utils/PropositionPower.sol	100.00% <100.00%> (ø)

[pscott]

It would be nice to move those _verify* to calldata and investigate those errors. Maybe we can create a tracking issue and still merge this #199 .

For _assertProposalExists, agreed the abstraction is probably worth it!

[Orland0x]

It would be nice to move those _verify* to calldata and investigate those errors. Maybe we can create a tracking issue and still merge this #199 .

For _assertProposalExists, agreed the abstraction is probably worth it!

could use structs its just a bit of a faf

[pscott]

tACK