-
Notifications
You must be signed in to change notification settings - Fork 46
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
[MAINTENANCE] Fix: check-deployed-spell
script
#419
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This fixes the problem with ETHERSCAN_API change, therefore approved.
But there is still problem with extra space that breaks two last checks on macOS:
[✔] DssSpell is verified.
[✔] DssSpell was verified with a valid license.
[✔] DssSpell solc version matches.
[✔] DssSpell was not compiled with optimizations.
[✔] DssSpell library matches hardcoded address in DssExecLib.address.
[✖] DssSpell deployment timestamp does not match.
[✖] DssSpell deployment block number does not match.
Do you also want to fix it in the same PR? Simply piping it into another sed to remove all spaces does the trick for me:
deployed_spell_block=$(grep -oE 'deployed_spell_block\s*:\s*[0-9]+' $CONFIG_PATH | sed -E 's/.*:\s*//' | sed -E 's/ //g')
deployed_spell_timestamp=$(grep -oE 'deployed_spell_created\s*:\s*[0-9]+' $CONFIG_PATH | sed -E 's/.*:\s*//' | sed -E 's/ //g')
Otherwise replacing sed with grep also works:
deployed_spell_block=$(grep -oE 'deployed_spell_block\s*:\s*[0-9]+' $CONFIG_PATH | grep -o '[0-9]\+')
deployed_spell_timestamp=$(grep -oE 'deployed_spell_created\s*:\s*[0-9]+' $CONFIG_PATH | grep -o '[0-9]\+')
BSD |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let me add it and test to see if nothing breaks on Linux.
We can one day add this script to the CI, but the CI would need be skipped in case of the empty address.
Anyway, "it works on my machine", therefore re-approved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(runs also on mine) approved
Description
Contribution Checklist
(PE-<TICKET_NUMBER>)
Checklist
officeHours
modifier override30 days
unless otherwise specified)ETH_GAS_LIMIT="XXX" ETH_GAS_PRICE="YYY" make deploy
mainnet
contract on etherscanmake archive-spell
ormake date="YYYY-MM-DD" archive-spell
to make an archive directory and copyDssSpell.sol
,DssSpell.t.sol
,DssSpell.t.base.sol
, andDssSpellCollateralOnboarding.sol
squash and merge
this PR