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

Enable partial transpilation for upgradeable package #4628

Merged
merged 36 commits into from
Sep 28, 2023
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
0f54291
enable partial transpilation
frangio Sep 27, 2023
b4a9a76
refactor prepare/prepack scripts
frangio Sep 27, 2023
f648bfe
add vanilla as dependency during upgradeable tests
frangio Sep 27, 2023
3910fbc
fix npm pack output
frangio Sep 27, 2023
270c006
fix npm pack
frangio Sep 27, 2023
df08dd7
fix transpile command
frangio Sep 27, 2023
59746e6
change approach without using pack
frangio Sep 27, 2023
e904e02
fix workflow
frangio Sep 27, 2023
0f2bafe
fix checkout location
frangio Sep 27, 2023
8002e8d
fix run
frangio Sep 27, 2023
9e2dcf1
fix mkdir
frangio Sep 27, 2023
0ff2901
fix ln
frangio Sep 27, 2023
5d8422f
add transpiler as dependency]
frangio Sep 27, 2023
de7a277
update hardhat-exposed
frangio Sep 27, 2023
63acb32
update transpiler
frangio Sep 27, 2023
5f0c801
enable exposed imports
frangio Sep 27, 2023
b6476c1
fix config
frangio Sep 27, 2023
448d8f6
add Stateless.sol
frangio Sep 27, 2023
0f7c3e2
fix stateless
frangio Sep 27, 2023
7246dbb
add dummy contract in stateless file
frangio Sep 27, 2023
ef88118
add all library imports
frangio Sep 27, 2023
01a768b
update hardhat-exposed
frangio Sep 27, 2023
e16c0ae
mark some contracts as stateless
Amxx Sep 28, 2023
9dc5442
sort
Amxx Sep 28, 2023
4771215
Add peer dependency to package.json
Amxx Sep 28, 2023
4ca92e2
fix import path
Amxx Sep 28, 2023
b320373
cleanup
Amxx Sep 28, 2023
268e17f
mark UUPSUpgradeable as stateless
Amxx Sep 28, 2023
781d5f2
Merge branch 'master' into partial-transpilation
frangio Sep 28, 2023
242b716
add changeset
frangio Sep 28, 2023
8d8030b
Update .github/workflows/checks.yml
frangio Sep 28, 2023
f63cf2d
simplify prepack
frangio Sep 28, 2023
86ebb2e
fix order of transpilation
frangio Sep 28, 2023
9e1ce8d
Merge branch 'master' into partial-transpilation
frangio Sep 28, 2023
b51a866
fix location where peer dependency is added
frangio Sep 28, 2023
5336454
read package version from contracts/package.json
Amxx Sep 28, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions .github/actions/setup/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,5 +15,3 @@ runs:
run: npm ci
shell: bash
if: steps.cache.outputs.cache-hit != 'true'
env:
SKIP_COMPILE: true
8 changes: 8 additions & 0 deletions .github/workflows/checks.yml
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,16 @@ jobs:
fetch-depth: 0 # Include history so patch conflicts are resolved automatically
- name: Set up environment
uses: ./.github/actions/setup
- name: Pack vanilla package
id: vanilla
run: |
tarball="$(npm pack --pack-destination .. | tee /dev/stderr | tail -1)"
echo "tarball=$tarball" >> "$GITHUB_OUTPUT"
working-directory: contracts
- name: Transpile to upgradeable
run: bash scripts/upgradeable/transpile.sh
- name: Install vanilla as dependency
run: npm install --no-save "${{ steps.vanilla.outputs.tarball }}"
- name: Run tests
run: npm run test
- name: Check linearisation of the inheritance graph
Expand Down
2 changes: 1 addition & 1 deletion contracts/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
"!/mocks/**/*"
],
"scripts": {
"prepare": "bash ../scripts/prepare-contracts-package.sh",
"prepack": "bash ../scripts/prepack.sh",
"prepare-docs": "cd ..; npm run prepare-docs"
},
"repository": {
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
"name": "openzeppelin-solidity",
"description": "Secure Smart Contract library for Solidity",
"version": "4.9.2",
"private": true,
"files": [
"/contracts/**/*.sol",
"/build/contracts/*.json",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Are we sure the build are getting in ?
The line above only gets solidity files

Copy link
Collaborator

Choose a reason for hiding this comment

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

actually, the entier "files" section of this package.json is probably useless

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the root package.json, that we don't publish any more. This "files" field probably doesn't matter but I preferred keeping it and just removing the directory that will no longer be there.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My reasoning is that there may be some workflow that relies on npm pack at the root, I don't know.

Expand All @@ -20,7 +21,6 @@
"lint:sol": "prettier --log-level warn --ignore-path .gitignore '{contracts,test}/**/*.sol' --check && solhint '{contracts,test}/**/*.sol'",
"lint:sol:fix": "prettier --log-level warn --ignore-path .gitignore '{contracts,test}/**/*.sol' --write",
"clean": "hardhat clean && rimraf build contracts/build",
"prepare": "scripts/prepare.sh",
"prepack": "scripts/prepack.sh",
"generate": "scripts/generate/run.js",
"release": "scripts/release/release.sh",
Expand Down
17 changes: 16 additions & 1 deletion scripts/prepack.sh
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,24 @@ set -euo pipefail
shopt -s globstar

# cross platform `mkdir -p`
node -e 'fs.mkdirSync("build/contracts", { recursive: true })'
mkdirp() {
node -e "fs.mkdirSync('$1', { recursive: true })"
}

# cd to the root of the repo
cd "$(git rev-parse --show-toplevel)"

npm run clean

env COMPILE_MODE=production npm run compile

mkdirp build/contracts
cp artifacts/contracts/**/*.json build/contracts
rm build/contracts/*.dbg.json

node scripts/remove-ignored-artifacts.js

cp README.md contracts/

mkdirp contracts/build/contracts
cp -r build/contracts/*.json contracts/build/contracts
Amxx marked this conversation as resolved.
Show resolved Hide resolved
15 changes: 0 additions & 15 deletions scripts/prepare-contracts-package.sh

This file was deleted.

10 changes: 0 additions & 10 deletions scripts/prepare.sh

This file was deleted.

4 changes: 3 additions & 1 deletion scripts/upgradeable/transpile.sh
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ fi
# -p: emit public initializer
# -n: use namespaces
# -N: exclude from namespaces transformation
# -q: partial transpilation using @openzeppelin/contracts as peer project
npx @openzeppelin/upgrade-safe-transpiler@latest -D \
-b "$build_info" \
-i contracts/proxy/utils/Initializable.sol \
Expand All @@ -36,7 +37,8 @@ npx @openzeppelin/upgrade-safe-transpiler@latest -D \
-x '!contracts/proxy/beacon/IBeacon.sol' \
-p 'contracts/**/presets/**/*' \
-n \
-N 'contracts/mocks/**/*'
-N 'contracts/mocks/**/*' \
-q '@openzeppelin/'
Copy link
Contributor Author

@frangio frangio Sep 27, 2023

Choose a reason for hiding this comment

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

@Amxx Does this look right to you? We have to do this because the contract source paths already have a contracts/ prefix, but it's not really true that @openzeppelin/ is the peer dependency. Ideally I would write something like @openzeppelin/contracts/=contracts/, like a Foundry remapping, but I don't want to complicate the transpiler more...

Copy link
Collaborator

Choose a reason for hiding this comment

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

I used to do -q '@openzeppelin/contracts/.. for my tests. What you wrote works just fine.

We can change to a remaping / remapings in a further version of the transpiler


# delete compilation artifacts of vanilla code
npm run clean
Loading