Skip to content

Conversation

@RonTuretzky
Copy link
Collaborator

No description provided.

RonTuretzky and others added 6 commits July 22, 2025 19:38
…150)

* Fix incorrect order of parent initializer calls in ButteredBread.sol

Reorder initializer calls to match the expected linearized order:
ERC20Upgradeable -> EIP712Upgradeable -> OwnableUpgradeable -> ReentrancyGuardUpgradeable

This resolves upgrade safety validation errors that were blocking deployment.

Fixes #149

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>

* Add missing EIP712 initializer call

The previous fix was incomplete - we also need to explicitly call __EIP712_init()
as it's not automatically called by __ERC20Votes_init().

Added:
- Import for EIP712Upgradeable
- __EIP712_init() call with contract name and version "1"

This ensures all parent initializers are called in the correct order.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>

---------

Co-authored-by: Claude <noreply@anthropic.com>
…-test

fix: use addr() to generate addresses from a seed value
Simplified remappings.txt from 9 entries to 6 and standardized all imports to use @OpenZeppelin patterns for better readability and consistency.

- Consolidated multiple OpenZeppelin remapping patterns into standardized @openzeppelin/contracts/ and @openzeppelin/contracts-upgradeable/ paths
- Updated 13 files across contracts, tests, and deployment scripts
- Maintained backward compatibility while improving code consistency
- All contracts compile successfully with new remappings

Fixes #130

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-authored-by: Claude <noreply@anthropic.com>
)

The function balanceOfLP was returning LPData struct (balance + scaling factor)
but the name suggested it only returned balance. Renamed to getLPData and
updated documentation to clarify it returns complete LP data structure.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-authored-by: Claude <noreply@anthropic.com>
@RonTuretzky RonTuretzky merged commit df26c4c into 141-automate-upgrade-safety-check-in-ci-pipeline Aug 9, 2025
RonTuretzky added a commit that referenced this pull request Aug 24, 2025
* Automate upgrade safety check in CI pipeline

- Update ValidateUpgrade.s.sol to validate both YieldDistributor and ButteredBread
- Create test/upgrades/ci/ directory for automated flattened contracts

Workflow file changes needed separately due to GitHub permissions.

Fixes #141

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>

* chore: ci/cd init

* Fix incorrect order of parent initializer calls in ButteredBread.sol

Reorder initializer calls to match the expected linearized order:
ERC20Upgradeable -> EIP712Upgradeable -> OwnableUpgradeable -> ReentrancyGuardUpgradeable

This resolves upgrade safety validation errors that were blocking deployment.

Fixes #149

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>

* Update flattened ButteredBread contract after merge

Regenerated flattened contract to include the initializer order fix from issue-149-fix.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>

* Simplify upgrade validation - remove contract flattening

- Updated ValidateUpgrade.s.sol to use validateImplementation instead of validateUpgrade
- Removed need for contract flattening and reference contracts
- Deleted test/upgrades/ci directory as it's no longer needed
- Skip storage checks since we're validating implementations, not upgrades

This approach is simpler and more maintainable for CI automation.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>

* Revert "Simplify upgrade validation - remove contract flattening"

This reverts commit d456847.

* Remove contract flattening from upgrade validation

- Removed flattening steps from CI workflow
- Updated ValidateUpgrade.s.sol to use v1.0.4 reference contracts
- Added unsafeSkipStorageCheck for YieldDistributor due to ERC7201 migration
- Removed test/upgrades/ci directory as it's no longer needed

The upgrade validation now works without flattening, making the CI simpler.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>

* Add automated contract flattening after validation passes

- Added 'flatten' job that runs only after tests and upgrade safety checks pass
- Flattening only occurs on pushes to dev/main branches (not on PRs)
- Contracts are flattened into versioned directories (test/upgrades/flattened/<version>/)
- Includes compilation verification of flattened contracts
- Auto-commits flattened contracts back to the repository with [skip ci] to avoid loops

This ensures flattened contracts are only created and tracked after all validation passes.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>

* update (#160)

* chore: Update .gitmodules to https

* Fix incorrect order of parent initializer calls in ButteredBread.sol (#150)

* Fix incorrect order of parent initializer calls in ButteredBread.sol

Reorder initializer calls to match the expected linearized order:
ERC20Upgradeable -> EIP712Upgradeable -> OwnableUpgradeable -> ReentrancyGuardUpgradeable

This resolves upgrade safety validation errors that were blocking deployment.

Fixes #149

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>

* Add missing EIP712 initializer call

The previous fix was incomplete - we also need to explicitly call __EIP712_init()
as it's not automatically called by __ERC20Votes_init().

Added:
- Import for EIP712Upgradeable
- __EIP712_init() call with contract name and version "1"

This ensures all parent initializers are called in the correct order.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>

---------

Co-authored-by: Claude <noreply@anthropic.com>

* fix: use addr() to generate addresses from a seed value

* Refactor OpenZeppelin library remappings (#151)

Simplified remappings.txt from 9 entries to 6 and standardized all imports to use @OpenZeppelin patterns for better readability and consistency.

- Consolidated multiple OpenZeppelin remapping patterns into standardized @openzeppelin/contracts/ and @openzeppelin/contracts-upgradeable/ paths
- Updated 13 files across contracts, tests, and deployment scripts
- Maintained backward compatibility while improving code consistency
- All contracts compile successfully with new remappings

Fixes #130

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-authored-by: Claude <noreply@anthropic.com>

* fix: rename balanceOfLP to getLPData to fix naming mismatch (#118) (#144)

The function balanceOfLP was returning LPData struct (balance + scaling factor)
but the name suggested it only returned balance. Renamed to getLPData and
updated documentation to clarify it returns complete LP data structure.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-authored-by: Claude <noreply@anthropic.com>

---------

Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: bagelface <bagelface@protonmail.com>
Co-authored-by: bagelface.eth <87501783+bagelface@users.noreply.github.com>

* update (#161)

* chore: Update .gitmodules to https

* Fix incorrect order of parent initializer calls in ButteredBread.sol (#150)

* Fix incorrect order of parent initializer calls in ButteredBread.sol

Reorder initializer calls to match the expected linearized order:
ERC20Upgradeable -> EIP712Upgradeable -> OwnableUpgradeable -> ReentrancyGuardUpgradeable

This resolves upgrade safety validation errors that were blocking deployment.

Fixes #149

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>

* Add missing EIP712 initializer call

The previous fix was incomplete - we also need to explicitly call __EIP712_init()
as it's not automatically called by __ERC20Votes_init().

Added:
- Import for EIP712Upgradeable
- __EIP712_init() call with contract name and version "1"

This ensures all parent initializers are called in the correct order.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>

---------

Co-authored-by: Claude <noreply@anthropic.com>

* fix: use addr() to generate addresses from a seed value

* Refactor OpenZeppelin library remappings (#151)

Simplified remappings.txt from 9 entries to 6 and standardized all imports to use @OpenZeppelin patterns for better readability and consistency.

- Consolidated multiple OpenZeppelin remapping patterns into standardized @openzeppelin/contracts/ and @openzeppelin/contracts-upgradeable/ paths
- Updated 13 files across contracts, tests, and deployment scripts
- Maintained backward compatibility while improving code consistency
- All contracts compile successfully with new remappings

Fixes #130

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-authored-by: Claude <noreply@anthropic.com>

* fix: rename balanceOfLP to getLPData to fix naming mismatch (#118) (#144)

The function balanceOfLP was returning LPData struct (balance + scaling factor)
but the name suggested it only returned balance. Renamed to getLPData and
updated documentation to clarify it returns complete LP data structure.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-authored-by: Claude <noreply@anthropic.com>

---------

Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: bagelface <bagelface@protonmail.com>
Co-authored-by: bagelface.eth <87501783+bagelface@users.noreply.github.com>

* Refactor upgrade validation to use flattened contracts

- Only flatten and track contracts on merges to dev branch (not main)
- Remove v1.0.4 hardcoded references, use flattened previous version
- Remove unsafeSkipStorageCheck flag from YieldDistributor validation
- Store flattened contracts in current/ and copy to previous/ for future validations
- Improve solc compilation output to show errors when compilation fails

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>

* Fix upgrade validation for initial deployment

- Add check in workflow to skip validation if no previous version exists
- Prevents CI failure when no flattened contracts are present yet
- Validation will run after first merge to dev creates initial flattened versions

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>

* Simplify flattened contracts workflow

- Remove solc compilation check from flatten step (forge already validates)
- Shorten paths from test/upgrades/flattened/ to test/upgrades/
- Update all references to use the shorter path structure

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>

* Add initial flattened contracts for upgrade validation

- Add flattened YieldDistributor and ButteredBread to test/upgrades/current/
- These will serve as the baseline for future upgrade validations
- Will be automatically updated on merges to dev branch

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>

* Apply bagelface's review feedback

- Switch from Foundry nightly to stable version
- Remove FOUNDRY_DISABLE_NIGHTLY_WARNING env var
- Add VotingMultipliers contract to upgrade validation
- Make contract flattening dynamic (flatten all top-level src/ contracts)
- Update validation check to be more generic (check for any .sol files)

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>

* Add release branches to PR workflow triggers

- Workflow now runs on PRs targeting release/** branches
- Ensures upgrade safety checks run for release PRs

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>

* Update workflow to run on any PR from branches containing 'release'

- Workflow now triggers on all PRs (not just to dev/main)
- Jobs check if PR is from a branch containing 'release' anywhere in the name
- Ensures release-related branches get full CI checks regardless of target

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>

* Fix workflow conditions to properly run on release branches

- Remove branch restrictions from pull_request trigger
- Add explicit conditions to run jobs on:
  - All pushes to dev/main
  - PRs targeting dev or main branches
  - PRs from any branch containing 'release' in the name
  - Manual workflow dispatch

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>

* Fix workflow to check target branch for 'release' as well

- Now checks both source branch (head_ref) and target branch (base_ref) for 'release'
- This handles PRs targeting release branches (e.g., PR to 164-release-v104)

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>

---------

Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: bagelface <bagelface@protonmail.com>
Co-authored-by: bagelface.eth <87501783+bagelface@users.noreply.github.com>
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