Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🔧 Standardize CI across active protocol repos #212

Conversation

neodaoist
Copy link
Contributor

No description provided.

@linear
Copy link

linear bot commented Jul 7, 2023

VAL-489 Standardize CI across active protocol repos

  • Remove hardcoded RPC (initially due to a GH secrets bug), esp if not using rn

@megsdevs
Copy link
Contributor

megsdevs commented Jul 7, 2023

https://github.com/valorem-labs-inc/clear/actions/runs/5481343531/jobs/9985521776?pr=212#step:6:332

Need to either fix the URI encoding, or disable the slither URI warning, either per line or globally. I assume you might not want to touch the solidity code so may be preferable to disable globally.

Not sure why slither didn't pick this up before it was changed for this commit? @0xAlcibiades

@megsdevs
Copy link
Contributor

megsdevs commented Jul 7, 2023

if you run slither locally it shows you the high priority warnings in red
Screenshot 2023-07-07 at 13 25 42

highly recommend using solc-select when using slither, this was the issue i was having to run locally before:
https://github.com/crytic/solc-select

then to select version use solc-select use 0.8.16 --always-install

@megsdevs
Copy link
Contributor

megsdevs commented Jul 7, 2023

although @0xAlcibiades @neodaoist can we check this isn't highlighting some unintended behaviour with the uri given the params are dynamic, it can cause clashes?

return string(
abi.encodePacked(
_escapeQuotes(params.underlyingSymbol),
_escapeQuotes(params.exerciseSymbol),
yearDigits[2],
yearDigits[3],
monthDigits.length == 2 ? monthDigits[0] : bytes1(uint8(48)),
monthDigits.length == 2 ? monthDigits[1] : monthDigits[0],
dayDigits.length == 2 ? dayDigits[0] : bytes1(uint8(48)),
dayDigits.length == 2 ? dayDigits[1] : dayDigits[0],
"C"
)
);
}

see https://github.com/crytic/slither/wiki/Detector-Documentation#abi-encodePacked-collision

@megsdevs megsdevs force-pushed the neo/val-489-standardize-ci-across-active-protocol-repos branch from ab8a486 to 6c48cf6 Compare July 10, 2023 13:17
@neodaoist neodaoist changed the base branch from master to release/v1.0.1 July 14, 2023 21:56
@codecov
Copy link

codecov bot commented Jul 14, 2023

Codecov Report

Merging #212 (63c717e) into release/v1.0.1 (d3458f1) will not change coverage.
The diff coverage is n/a.

❗ Current head 63c717e differs from pull request most recent head 8a859cd. Consider uploading reports for the commit 8a859cd to get more accurate results

@@               Coverage Diff               @@
##           release/v1.0.1     #212   +/-   ##
===============================================
  Coverage           99.20%   99.20%           
===============================================
  Files                   1        1           
  Lines                 250      250           
  Branches               41       41           
===============================================
  Hits                  248      248           
  Partials                2        2           

@neodaoist neodaoist merged commit 6498fe7 into release/v1.0.1 Jul 14, 2023
@neodaoist neodaoist deleted the neo/val-489-standardize-ci-across-active-protocol-repos branch July 14, 2023 22:00
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