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

feat: skip new contracts on check_storage_layout #974

Draft
wants to merge 37 commits into
base: development
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 36 commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
e604f13
feat: remove newly added contracts from core storage change check wor…
zugdev Oct 3, 2024
d75ad0f
feat: add a new contract to test core contracts storage check workflow
zugdev Oct 4, 2024
be48114
feat: add a new variable with collision to test core contracts storag…
zugdev Oct 4, 2024
79c41d6
feat: read env to set-matrix workflow step
zugdev Oct 4, 2024
4911b86
feat: add a new variable with collision to test workflow
zugdev Oct 4, 2024
034c8b4
feat: add a new variable without colision to test storage workflow
zugdev Oct 4, 2024
486386e
feat: add another variable with collision to check storage check work…
zugdev Oct 4, 2024
09bb1bf
feat: add random comments to test storage check workflow
zugdev Oct 4, 2024
76732cb
chore: delete test workflow contract
zugdev Oct 4, 2024
9559223
feat: remove fork branch from branches in workflow
zugdev Oct 4, 2024
9ed317f
feat: add new contract storage check should skip
zugdev Oct 4, 2024
18623bb
feat: fix storage check workflow to check against development branch
zugdev Oct 4, 2024
a7e0375
chore: remove workflow test contract
zugdev Oct 4, 2024
7bb1c7b
feat: test storage check workflow contract creation should skip
zugdev Oct 4, 2024
a6477bc
feat: add a new variable to new contract should skip
zugdev Oct 4, 2024
8113e5e
feat: modify a core contract to test storage check workflow should no…
zugdev Oct 5, 2024
58ca4e4
feat: fix char put out in workflow
zugdev Oct 5, 2024
6513217
feat: modify a core contract with collision to test storage check wor…
zugdev Oct 5, 2024
6c698c9
feat: remove test workflow changes from core contract
zugdev Oct 5, 2024
f344231
feat: add a non colliding variable to core contract to test storage c…
zugdev Oct 5, 2024
f217ecb
feat: remove workflow test variable
zugdev Oct 5, 2024
6f1f50f
feat: test storage check on no storage update
zugdev Oct 5, 2024
ed7f656
feat: remove testing comments
zugdev Oct 5, 2024
61bba9d
test: delete workflow testing contract
zugdev Oct 16, 2024
b601e97
feat: skip new constracts in check diamond storage
zugdev Oct 16, 2024
69d35e9
test(storage-workflows): should skip new contract and new lib
zugdev Oct 16, 2024
9c940e8
test(storage-workflows): should skip new contract even after first co…
zugdev Oct 16, 2024
2953cb8
test(storage-workflow): should fail if core / diamond storage is chan…
zugdev Oct 16, 2024
88fa459
test(storage-workflow): should fail if core / diamond storage is chan…
zugdev Oct 16, 2024
ad8b124
test(storage-workflow): should fail if core / diamond storage is chan…
zugdev Oct 16, 2024
f8b5c3f
test(storage-workflow): should pass if core / diamond storage is chan…
zugdev Oct 17, 2024
4bdfffa
feat: add a workflow to generate storage artifacts
zugdev Oct 17, 2024
b44ea46
fix: storage artifacts generation workflow
zugdev Oct 17, 2024
c95eaa0
feat: polish storage-check
zugdev Oct 19, 2024
f592796
Merge branch 'development' of github.com:ubiquity/ubiquity-dollar int…
zugdev Jan 31, 2025
9462e2f
feat: diamond use rubilmax
zugdev Feb 2, 2025
d0eb82f
feat: rollback
zugdev Feb 2, 2025
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
28 changes: 21 additions & 7 deletions .github/workflows/core-contracts-storage-check.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,10 @@ jobs:
runs-on: ubuntu-latest

steps:
- uses: actions/checkout@v3
- uses: actions/checkout@v4
with:
submodules: recursive

- name: Install Foundry
uses: onbjerg/foundry-toolchain@v1
with:
Expand All @@ -35,10 +35,25 @@ jobs:
env:
CHANGED_CONTRACTS: ${{ steps.changed-contracts.outputs.contracts_all_changed_files }}
run: |
for CONTRACT in "$CHANGED_CONTRACTS"; do
echo ${CONTRACT} | xargs basename -a | cut -d'.' -f1 | xargs -I{} echo src/dollar/core/{}.sol:{} >> contracts.txt
touch contracts.txt

# Fetch the latest state of the development branch to compare
git fetch origin development

# Iterate through changed contracts and check if they existed in the development branch
for CONTRACT in $CHANGED_CONTRACTS; do
if git show origin/development:$CONTRACT > /dev/null 2>&1; then
# If the contract existed in the development branch, add it to the list for storage check
echo ${CONTRACT} | xargs basename -a | cut -d'.' -f1 | xargs -I{} echo src/dollar/core/{}.sol:{} >> contracts.txt
else
echo "$CONTRACT is a new contract, skipping storage check."
fi
done
echo "matrix=$(cat contracts.txt | jq -R -s -c 'split("\n")[:-1]')" >> $GITHUB_OUTPUT

# Set the matrix only if there are contracts to check
if [ -s contracts.txt ]; then
echo "matrix=$(cat contracts.txt | jq -R -s -c 'split("\n")[:-1]')" >> $GITHUB_OUTPUT
fi

outputs:
matrix: ${{ steps.set-matrix.outputs.matrix }}
Expand Down Expand Up @@ -67,5 +82,4 @@ jobs:
with:
workingDirectory: packages/contracts
contract: ${{ matrix.contract }}
failOnRemoval: true

failOnRemoval: true
30 changes: 22 additions & 8 deletions .github/workflows/diamond-storage-check.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ jobs:
runs-on: ubuntu-latest

steps:
- uses: actions/checkout@v3
- uses: actions/checkout@v4
with:
submodules: recursive

Expand All @@ -39,10 +39,25 @@ jobs:
env:
CHANGED_LIBS: ${{ steps.changed-libraries.outputs.libraries_all_changed_files }}
run: |
for DIAMOND_LIB in "$CHANGED_LIBS"; do
echo ${DIAMOND_LIB} | xargs basename -a | cut -d'.' -f1 | xargs -I{} echo src/dollar/libraries/{}.sol:{} >> contracts.txt
touch contracts.txt

# Fetch the latest state of the development branch to compare
git fetch origin development

# Iterate through changed libraries and check if they existed in the development branch
for DIAMOND_LIB in $CHANGED_LIBS; do
if git show origin/development:$DIAMOND_LIB > /dev/null 2>&1; then
# If the library existed in the development branch, add it to the list for storage check
echo ${DIAMOND_LIB} | xargs basename -a | cut -d'.' -f1 | xargs -I{} echo src/dollar/libraries/{}.sol:{} >> contracts.txt
else
echo "$DIAMOND_LIB is a new library, skipping storage check."
fi
done
echo "matrix=$(cat contracts.txt | jq -R -s -c 'split("\n")[:-1]')" >> $GITHUB_OUTPUT

# Set the matrix only if there are libraries to check
if [ -s contracts.txt ]; then
echo "matrix=$(cat contracts.txt | jq -R -s -c 'split("\n")[:-1]')" >> $GITHUB_OUTPUT
fi

outputs:
matrix: ${{ steps.set-matrix.outputs.matrix }}
Expand All @@ -66,10 +81,9 @@ jobs:
with:
version: nightly

- name: Check For Diamond Storage Changes
uses: ubiquity/foundry-storage-check@main
- name: Check For Core Contracts Storage Changes
uses: Rubilmax/foundry-storage-check@main
Copy link
Member

Choose a reason for hiding this comment

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

Probably more appropriate to update our fork.

Copy link
Contributor Author

@zugdev zugdev Feb 3, 2025

Choose a reason for hiding this comment

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

Actually the problem is forge inspect <CONTRACT_NAME> storageLayout, which is used by Rubilmax returns null for libraries, so our fork manually creates a layout storage for the libraries. Apparently it might be broken. Therefore, we have to fix our fork, using that action won't really work.

Copy link
Member

Choose a reason for hiding this comment

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

we have to fix our fork

Yes that's what I meant

with:
workingDirectory: packages/contracts
contract: ${{ matrix.contract }}
failOnRemoval: true
failOnLabelDiff: true
failOnRemoval: true
Loading