Skip to content

[v1.5] Polymorphic Price Config [WIP]#135

Merged
Whytecrowe merged 39 commits intorc/zchain-native-mainfrom
feat/polymorphic-pricer-configs
Jun 5, 2025
Merged

[v1.5] Polymorphic Price Config [WIP]#135
Whytecrowe merged 39 commits intorc/zchain-native-mainfrom
feat/polymorphic-pricer-configs

Conversation

@JamesEarle
Copy link
Copy Markdown
Contributor

@JamesEarle JamesEarle commented Apr 25, 2025

Change the price contracts to be stateless and use given bytes objects instead when calculating prices and fees. This allows domain registration and setup for selling subdomains to happen in a single transaction. Updated to include subregistrar changes in upgrade mock contracts

@openzeppelin-code
Copy link
Copy Markdown

openzeppelin-code bot commented Apr 25, 2025

[v1.5] Polymorphic Price Config [WIP]

Generated at commit: 046f3ff77f1c164c83d909119676a5c7060e23c3

🚨 Report Summary

Severity Level Results
Contracts Critical
High
Medium
Low
Note
Total
1
1
0
2
23
27
Dependencies Critical
High
Medium
Low
Note
Total
0
0
0
0
0
0

For more details view the full report in OpenZeppelin Code Inspector

@Whytecrowe Whytecrowe changed the title [DRAFT] Polymorphic Price Config [v1.5] [DRAFT] Polymorphic Price Config May 7, 2025
@Whytecrowe Whytecrowe changed the title [v1.5] [DRAFT] Polymorphic Price Config [v1.5] Polymorphic Price Config [WIP] May 19, 2025
Copy link
Copy Markdown
Contributor

@Whytecrowe Whytecrowe left a comment

Choose a reason for hiding this comment

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

please, address comments

@JamesEarle JamesEarle requested a review from Copilot May 22, 2025 22:26
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This pull request updates the price contracts to be stateless by removing deployment argument functions and relying on provided bytes objects for price and fee calculations.

  • Removed the deployArgs functions and updated the proxyData configuration in both fixedPricer and curvePricer contracts.
  • Updated configuration defaults and removed the extra isSet flag from the priceConfig in get-config.
  • Upgraded Solidity versions for proxy contracts in the Hardhat config and aligned documentation in the related markdown file.

Reviewed Changes

Copilot reviewed 29 out of 44 changed files in this pull request and generated no comments.

Show a summary per file
File Description
src/deploy/missions/contracts/fixed-pricer.ts Removal of deployArgs and change to proxyData (isProxy false only).
src/deploy/missions/contracts/curve-pricer.ts Removal of deployArgs and change to proxyData (isProxy false but retains kind).
src/deploy/campaign/get-config.ts Replaced DEFAULT_PRICE_CONFIG with DEFAULT_CURVE_PRICE_CONFIG and removed isSet property in priceConfig.
hardhat.config.ts Upgraded OpenZeppelin contract versions from 0.8.20 to 0.8.22.
docs/contracts/price/IZNSFixedPricer.md Removed the isSet field from the PriceConfig struct in documentation.
Files not reviewed (15)
  • contracts/price/IZNSCurvePricer.sol: Language not supported
  • contracts/price/IZNSFixedPricer.sol: Language not supported
  • contracts/price/IZNSPricer.sol: Language not supported
  • contracts/price/ZNSCurvePricer.sol: Language not supported
  • contracts/price/ZNSFixedPricer.sol: Language not supported
  • contracts/registrar/IDistributionConfig.sol: Language not supported
  • contracts/registrar/IZNSRootRegistrar.sol: Language not supported
  • contracts/registrar/IZNSSubRegistrar.sol: Language not supported
  • contracts/registrar/ZNSRootRegistrar.sol: Language not supported
  • contracts/registrar/ZNSSubRegistrar.sol: Language not supported
  • contracts/registry/IZNSRegistry.sol: Language not supported
  • contracts/registry/ZNSRegistry.sol: Language not supported
  • contracts/types/ICurvePriceConfig.sol: Language not supported
  • contracts/upgrade-test-mocks/distribution/ZNSSubRegistrarMock.sol: Language not supported
  • contracts/utils/CommonErrors.sol: Language not supported
Comments suppressed due to low confidence (4)

src/deploy/missions/contracts/fixed-pricer.ts:15

  • Removal of the deployArgs function changes the contract deployment API; please ensure all related deployment processes and documentation are updated accordingly.
async deployArgs () : Promise<TDeployArgs> {

src/deploy/missions/contracts/curve-pricer.ts:16

  • Removal of the deployArgs function impacts the deployment interface; verify that all deployment scripts and related references have been adapted to this change.
async deployArgs () : Promise<TDeployArgs> {

src/deploy/campaign/get-config.ts:247

  • The removal of the 'isSet' property from the returned priceConfig may affect consumers expecting this property; please confirm this change aligns with the new stateless design.
isSet: true,

src/deploy/missions/contracts/curve-pricer.ts:18

  • [nitpick] Consider whether including the 'kind' key in proxyData is necessary when isProxy is false to maintain consistency with the fixedPricer contract.
proxyData = { isProxy: false, kind: ProxyKinds.uups, };

@JamesEarle JamesEarle requested a review from Copilot May 24, 2025 00:29
@JamesEarle JamesEarle force-pushed the feat/polymorphic-pricer-configs branch from 0e1b1ad to 993ada6 Compare May 24, 2025 00:30
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors pricing contracts to be stateless by passing serialized config bytes, removes proxy deployments for pricers, updates default curve price config usage, and bumps compiler overrides.

  • Introduces DEFAULT_CURVE_PRICE_CONFIG_BYTES for root registrar initialization
  • Disables UUPS proxy for fixed and curve pricers and removes their custom deploy arguments
  • Switches to DEFAULT_CURVE_PRICE_CONFIG in config loader and removes deprecated flags
  • Upgrades OpenZeppelin compiler overrides from 0.8.20 to 0.8.22

Reviewed Changes

Copilot reviewed 29 out of 43 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/deploy/missions/contracts/root-registrar.ts Add byte-serialized config to root registrar constructor args
src/deploy/missions/contracts/fixed-pricer.ts Disable proxy deployment and remove deployArgs override
src/deploy/missions/contracts/curve-pricer.ts Disable proxy deployment and remove deployArgs override
src/deploy/campaign/get-config.ts Replace default price config references and drop isSet flag
hardhat.config.ts Bump OZ proxy compiler overrides to 0.8.22
docs/contracts/price/IZNSFixedPricer.md Remove obsolete isSet field from documentation
Files not reviewed (14)
  • contracts/price/IZNSCurvePricer.sol: Language not supported
  • contracts/price/IZNSFixedPricer.sol: Language not supported
  • contracts/price/IZNSPricer.sol: Language not supported
  • contracts/price/ZNSCurvePricer.sol: Language not supported
  • contracts/price/ZNSFixedPricer.sol: Language not supported
  • contracts/registrar/IDistributionConfig.sol: Language not supported
  • contracts/registrar/IZNSRootRegistrar.sol: Language not supported
  • contracts/registrar/IZNSSubRegistrar.sol: Language not supported
  • contracts/registrar/ZNSRootRegistrar.sol: Language not supported
  • contracts/registrar/ZNSSubRegistrar.sol: Language not supported
  • contracts/registry/ZNSRegistry.sol: Language not supported
  • contracts/types/ICurvePriceConfig.sol: Language not supported
  • contracts/upgrade-test-mocks/distribution/ZNSSubRegistrarMock.sol: Language not supported
  • contracts/utils/CommonErrors.sol: Language not supported
Comments suppressed due to low confidence (2)

src/deploy/campaign/get-config.ts:249

  • The isSet flag was removed from the returned config object, but downstream code or type definitions might still expect it; ensure the ICurvePriceConfig type and consumers are updated accordingly.
isSet: true,

src/deploy/missions/contracts/fixed-pricer.ts:18

  • [nitpick] The kind: ProxyKinds.uups property remains in proxyData even though isProxy is false; consider removing the unused kind field or the entire proxyData block if proxies are no longer used.
isProxy: false,

@codecov
Copy link
Copy Markdown

codecov bot commented May 24, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.46%. Comparing base (516eade) to head (046f3ff).
Report is 40 commits behind head on rc/zchain-native-main.

Additional details and impacted files
@@                    Coverage Diff                    @@
##           rc/zchain-native-main     #135      +/-   ##
=========================================================
- Coverage                  99.50%   99.46%   -0.05%     
=========================================================
  Files                         12       12              
  Lines                        608      560      -48     
  Branches                     136      120      -16     
=========================================================
- Hits                         605      557      -48     
  Misses                         3        3              
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Copy Markdown
Contributor

@Whytecrowe Whytecrowe left a comment

Choose a reason for hiding this comment

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

comments

@Whytecrowe Whytecrowe merged commit dbf27d1 into rc/zchain-native-main Jun 5, 2025
3 checks passed
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