-
Notifications
You must be signed in to change notification settings - Fork 73
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
Refactor dependencies to use latest versions and update CI workflows for safe-modules-allowance. #490
base: main
Are you sure you want to change the base?
Conversation
…for safe-modules-allowance.
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.
LGTM, but CI is failing.
…acts version 1.4.1.
modules/allowances/package.json
Outdated
"author": "[email protected]", | ||
"license": "ISC", | ||
"author": "safe-global", | ||
"license": "GPL-3.0", |
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.
FYI: GPL-3.0 is deprecated. https://spdx.org/licenses/GPL-3.0.html
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 is the one we use in the other files, I'd raise this with legal. It says the identifier has been deprecated but not the license. We need a translation from legalese
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.
I changed it to GPL because this is the one we use in other files. I see it's deprecated and asked in Slack which one we should use.
Let's address this later in a different PR (I can also revert the change until we get more clarity)
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 is the one we use in the other files, I'd raise this with legal. It says the identifier has been deprecated but not the license. We need a translation from legalese
The non-deprecated identifier is GPL-3.0-only
.
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.
Basically, the GPL-3.0
identifier was ambiguous because GPL has a "license update clause" (meaning that if they come up with a GPL-4, then all GPL-3 code will also be licensed under v4). In SPDX, this was represented by GPL-3.0-or-later
. GPL-3.0
changed to GPL-3.0-only
to avoid confusion (as it is the GPL license opting out of the "license update clause").
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.
Looking at the contract code, this should be LGPL-3.0-only
(see the SPDX identifier used in the Solidity code).
…ctories. * Remove unnecessary directory references in Safe4337Module.conf * Update package paths in SignatureLengthCheck.conf, TransactionExecutionMethods.conf and ValidationDataLastBitOne.conf * Refactor checkSignatures function in Account.sol to be viewless
@@ -15,9 +15,10 @@ | |||
], | |||
"rule_sanity": "basic", | |||
"solc": "solc8.23", | |||
"solc_allow_path":"../../node_modules", |
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.
Previously there was a bug with solc's allow path option and certora-cli, now it's fixed we can use it and get rid of the crazy direct links
"@account-abstraction=../../node_modules/.pnpm/@[email protected]/node_modules/@account-abstraction", | ||
"@safe-global=../../node_modules/.pnpm/@[email protected][email protected][email protected][email protected]_/node_modules/@safe-global" | ||
"@account-abstraction=node_modules/@account-abstraction", | ||
"@safe-global=node_modules/@safe-global" |
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.
🎉
Pull Request Test Coverage Report for Build 11324077193Details
💛 - Coveralls |
I updated the licenses everywhere to be LGPL-3.0 only, except the recovery module, because I wasn't sure if we could change the license since it's a distribution of another project. |
a71cbef
to
fbc6511
Compare
This PR:
ISafe
used in tests and in the production codepnpm
in the Safe Allowance module