Skip to content

Conversation

@rayedsikder
Copy link
Member

@rayedsikder rayedsikder commented Apr 21, 2025

Architecture Redesign: ERC1167 Minimal Proxy using clones, Improved Contract Initializations, and chore upgrades

This PR introduces significant improvements to the Creative Crowdfunding Protocol contracts architecture with cleaner code organization, improved deployment workflows, and enhanced security features.

Key Changes

Architecture Improvements

  • Refactored contract inheritance patterns for better code reuse and modularity
  • Enhanced treasury implementation with a cleaner ERC1167 Minimal Proxy using clones instead of CREATE2
  • Implemented initialization pattern for proxied contracts replacing constructor-heavy approach
  • Added cancellation functionality alongside pause mechanisms for better campaign state control
  • Added proper initializer modifier to prevent re-initialization vulnerabilities

Documentation & Environment

  • Completely restructured documentation with improved navigation and readability
  • Added comprehensive deployment scripts using Foundry's script framework
  • Updated environment configuration for more flexible deployment options
  • Updated config constants for better maintainability

Security Enhancements

  • Renamed platform identifiers from platformBytes to platformHash for better semantic clarity
  • Simplified treasury factory implementation using OpenZeppelin proxies
  • Consolidated access control patterns across contracts
  • Implemented Solidity 0.8.20 features for better security guarantees
  • Added custom errors for improved exception handling and gas efficiency
  • Improved validation logic throughout contracts

Treasury Model Updates

  • Consolidated treasury implementations with improved inheritance
  • Enhanced reward management functionality with improved batching
  • Fixed critical issue in claimRefund to properly handle pledge amounts and fees
  • Improved campaign state transitions (launch, pause, cancel)
  • Renamed internal variables for better semantic clarity (e.g., s_tokenToTotalCollectedAmount)
  • Made function names more consistent with their logic
  • Removed redundant functions and consolidated similar functionality

Breaking Changes

  • Treasury factory API has significant changes in method signatures and behavior
  • Platform identifier parameter names have changed across interfaces
  • Deployment process now uses a different pattern with initialization functions
  • Updated reward management functions with different signatures

Please review especially the treasury factory implementation and the deployment scripts, as these represent the most significant architectural changes, as well as the refactored refund logic which fixes several accounting issues.

- Pausing a campaign
- Cancelling a campaign
- Clonable with Immutable Args
- Update imports
- Replace `platformBytes` with `platformHash`
- Update function names
- Remove deterministic create2 deployment
- Remove bytecode deployment methods
- Introduce ERC1167 minimal proxy
- Add PausableCancellable functions
- Refactor Reward struct
- Update ERC20 with safeERC20
- Add feature to accept shipping fee
- Upgrade constructor with Initializer
- Remove fiat relevant functions and calculations in raised amounts
- Remove preLaunch pledge functions
@rayedsikder rayedsikder added the enhancement New feature or request label Apr 21, 2025
@rayedsikder rayedsikder self-assigned this Apr 21, 2025
Copy link
Collaborator

@mahabubAlahi mahabubAlahi left a comment

Choose a reason for hiding this comment

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

Besides my comments, all the contracts LGTM.

- Removed addReward
- Improved validation logic in `addRewards`
- Combined the if conditions to make it concise
- Updated the function visibily of `_pledge`
- `Reordered _checkSuccessCondition`
- Added `s_tokenToPledgedAmount` to map pledged amount per token ID
- Renamed `s_tokenToCollectedAmount` to `s_tokenToTotalCollectedAmount`
- Update `claimRefund` function to refund full amount including pledged food and shipping fees.
- Fixed issue in `claimRefund` `s_pledgedAmount` balance deduction
Copy link
Collaborator

@mahabubAlahi mahabubAlahi left a comment

Choose a reason for hiding this comment

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

  • For consistency, two require checking still exist in the ItemRegistry.sol and Counters.sol; they can be replaced with custom errors.

LGTM

@rayedsikder rayedsikder merged commit be34f00 into main Apr 22, 2025
1 check failed
@rayedsikder rayedsikder changed the title Architecture Redesign: Treasury Factory Pattern & Improved Contract Initialization ERC1167 Minimal Proxy using clones, Improved Contract Initializations, and chore upgrades Apr 22, 2025
@rayedsikder rayedsikder changed the title ERC1167 Minimal Proxy using clones, Improved Contract Initializations, and chore upgrades Architecture Redesign: ERC1167 Minimal Proxy using clones, Improved Contract Initializations, and chore upgrades Apr 22, 2025
rayedsikder pushed a commit that referenced this pull request Dec 9, 2025
* Fix incorrect description of the `claimFund` function (#6)

  (immunefi)(issue#06)
  - Updated the NatSpec comment for claimFund()

* Refactor `platformDataKey` parameter validation optimization (#7)

  (immunefi)(issue#09)
  - Move `platformDataKey` parameter validation to an earlier stage of the `createCampaign` flow

* Add configuring platform data during `updateSelectedPlatform` (#8)

  (immunefi)(issue#10)

* Add creator's non-zero validation during `createCampaign` (#9)

  (immunefi)(issue#16)

* Add zero validation for `platformDataValue` in `createCampaign` (#10)

  (immunefi)(issue#12)

* Add reward value zero validation in pledge (#11)

  (immunefi)(issue#14)

* Fix `updateDeadline` allowing past deadline that blocks `claimRefund` (#12)

  (immunefi)(issue#05)
  - Added check to ensure new deadline is after current block timestamp

* Fix blocking KeepWhatsRaised pledge functions via front-running (#13)

  (immunefi)(issue#04)
  - Add internal pledge ID generation using msg.sender and pledgeId

* Add fee configuration via configure treasury (#14)

  (immunefi)(issue#11)
  - Update configure treasury to support fee values
  - Add getter function for fee value

* Add campaign data validation in configure treasury (#15)

  (immunefi)(issue#20)
  - Update fee values js doc
  - Update custom error

* Fix Gateway fee bypass (#16)

  (immunefi)(issue#19)
  - When `setFeeAndPledge` is called, tokens are transferred from the admin's wallet (msg.sender)
  - When `pledgeWithoutAReward` or `pledgeForAReward` is called directly, tokens are transferred from the backer's wallet

* Add expected fee description in create campaign jsdoc (#17)

  (immunefi)(issue#03)

* Refactor withdrawal and pledge calculation (#19)

  (immunefi)(issue#15)
  (immunefi)(issue#18)

  - Restrict access to the withdrawal function so that only the campaign owner and platform administrator can use it.
  - Move the protocol fee calculation from the withdrawal process to the pledge stage.
  - For withdrawals:
    - Partial Withdrawals:
       - Conditions: amount > 0 and amount + fees ≤ available balance.
       - The creator must specify the exact withdrawal amount, and the system will ensure there are sufficient funds to cover both the withdrawal and the fees.

    - Final Withdrawals:
      - Conditions: available balance > 0 and fees ≤ available balance.
      - The creator can withdraw the entire remaining balance. The system will check if there are enough funds to cover the fees and will then provide the remainder to the creator.

---------

Co-authored-by: mahabubAlahi <mahabub@ccprotocol.xyz>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants