From 5f601cb58f7af52c33668c733a3dc89902f65bc3 Mon Sep 17 00:00:00 2001 From: James Earle Date: Wed, 25 Oct 2023 14:31:56 -0400 Subject: [PATCH 1/6] botonm out in pricing so can't go below minPrice --- contracts/price/ZNSCurvePricer.sol | 7 +++++-- hardhat.config.ts | 2 +- test/ZNSCurvePricer.test.ts | 18 ++++++++++++++++++ 3 files changed, 24 insertions(+), 3 deletions(-) diff --git a/contracts/price/ZNSCurvePricer.sol b/contracts/price/ZNSCurvePricer.sol index a5a647c00..ff54866a7 100644 --- a/contracts/price/ZNSCurvePricer.sol +++ b/contracts/price/ZNSCurvePricer.sol @@ -291,9 +291,12 @@ contract ZNSCurvePricer is AAccessControlled, ARegistryWired, UUPSUpgradeable, I if (length <= config.baseLength) return config.maxPrice; if (length > config.maxLength) return config.minPrice; - return - (config.baseLength * config.maxPrice / length) + uint256 price = (config.baseLength * config.maxPrice / length) / config.precisionMultiplier * config.precisionMultiplier; + + if(price < config.minPrice) return config.minPrice; + + return price; } /** diff --git a/hardhat.config.ts b/hardhat.config.ts index a93f8c633..354b135bf 100644 --- a/hardhat.config.ts +++ b/hardhat.config.ts @@ -71,7 +71,7 @@ const config : HardhatUserConfig = { timeout: 5000000, }, gasReporter: { - enabled: true + enabled: false }, networks: { mainnet: { diff --git a/test/ZNSCurvePricer.test.ts b/test/ZNSCurvePricer.test.ts index 8dd6b0c48..6d61e2a3c 100644 --- a/test/ZNSCurvePricer.test.ts +++ b/test/ZNSCurvePricer.test.ts @@ -100,6 +100,24 @@ describe("ZNSCurvePricer", () => { }); describe("#getPrice", async () => { + it("Cannot go below the set minPrice", async () => { + const newConfig = { + baseLength: BigNumber.from("5"), + maxLength: BigNumber.from("10"), + maxPrice: parseEther("10"), + minPrice: parseEther("5.5"), + precisionMultiplier: precisionMultiDefault, + feePercentage: registrationFeePercDefault, + }; + + // as a user of "domainHash" that's not 0x0 + await zns.curvePricer.connect(user).setPriceConfig(domainHash, newConfig); + + let name = "abcdefghij" // length 10 + const price = await zns.curvePricer.getPrice(domainHash, name); + expect(price).to.eq(newConfig.minPrice); + }); + it("Returns 0 price for a root name with no length", async () => { const { price, From 46704d011342a4a43a6c9c5e8db9d05f7d01c54b Mon Sep 17 00:00:00 2001 From: James Earle Date: Wed, 25 Oct 2023 16:13:19 -0400 Subject: [PATCH 2/6] recommended fix for price drop before minPrice in domain length --- contracts/price/ZNSCurvePricer.sol | 2 +- test/ZNSCurvePricer.test.ts | 34 ++++++++++++++---------------- 2 files changed, 17 insertions(+), 19 deletions(-) diff --git a/contracts/price/ZNSCurvePricer.sol b/contracts/price/ZNSCurvePricer.sol index ff54866a7..8d48f20d3 100644 --- a/contracts/price/ZNSCurvePricer.sol +++ b/contracts/price/ZNSCurvePricer.sol @@ -307,7 +307,7 @@ contract ZNSCurvePricer is AAccessControlled, ARegistryWired, UUPSUpgradeable, I * which can occur if some of the config values are not properly chosen and set. */ function _validateConfig(bytes32 domainHash) internal view { - uint256 prevToMinPrice = _getPrice(domainHash, priceConfigs[domainHash].maxLength - 1); + uint256 prevToMinPrice = _getPrice(domainHash, priceConfigs[domainHash].maxLength); require( priceConfigs[domainHash].minPrice <= prevToMinPrice, "ZNSCurvePricer: incorrect value set causes the price spike at maxLength." diff --git a/test/ZNSCurvePricer.test.ts b/test/ZNSCurvePricer.test.ts index 6d61e2a3c..b45d88efb 100644 --- a/test/ZNSCurvePricer.test.ts +++ b/test/ZNSCurvePricer.test.ts @@ -100,24 +100,6 @@ describe("ZNSCurvePricer", () => { }); describe("#getPrice", async () => { - it("Cannot go below the set minPrice", async () => { - const newConfig = { - baseLength: BigNumber.from("5"), - maxLength: BigNumber.from("10"), - maxPrice: parseEther("10"), - minPrice: parseEther("5.5"), - precisionMultiplier: precisionMultiDefault, - feePercentage: registrationFeePercDefault, - }; - - // as a user of "domainHash" that's not 0x0 - await zns.curvePricer.connect(user).setPriceConfig(domainHash, newConfig); - - let name = "abcdefghij" // length 10 - const price = await zns.curvePricer.getPrice(domainHash, name); - expect(price).to.eq(newConfig.minPrice); - }); - it("Returns 0 price for a root name with no length", async () => { const { price, @@ -325,6 +307,22 @@ describe("ZNSCurvePricer", () => { ).to.be.revertedWith(CURVE_PRICE_CONFIG_ERR); }); + it("Cannot go below the set minPrice", async () => { + // Using config numbers from audit + const newConfig = { + baseLength: BigNumber.from("5"), + maxLength: BigNumber.from("10"), + maxPrice: parseEther("10"), + minPrice: parseEther("5.5"), + precisionMultiplier: precisionMultiDefault, + feePercentage: registrationFeePercDefault, + }; + + await expect( + zns.curvePricer.connect(user).setPriceConfig(domainHash, newConfig) + ).to.be.revertedWith(CURVE_PRICE_CONFIG_ERR); + }); + it("Should revert if called by anyone other than owner or operator", async () => { const newConfig = { baseLength: BigNumber.from("6"), From a7f86ebd0266c79928f3310564b55d0c8fba51db Mon Sep 17 00:00:00 2001 From: James Earle Date: Wed, 25 Oct 2023 16:55:55 -0400 Subject: [PATCH 3/6] remove price check on return --- contracts/price/ZNSCurvePricer.sol | 6 +----- test/ZNSCurvePricer.test.ts | 6 ++++++ 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/contracts/price/ZNSCurvePricer.sol b/contracts/price/ZNSCurvePricer.sol index 8d48f20d3..80f38d39c 100644 --- a/contracts/price/ZNSCurvePricer.sol +++ b/contracts/price/ZNSCurvePricer.sol @@ -291,12 +291,8 @@ contract ZNSCurvePricer is AAccessControlled, ARegistryWired, UUPSUpgradeable, I if (length <= config.baseLength) return config.maxPrice; if (length > config.maxLength) return config.minPrice; - uint256 price = (config.baseLength * config.maxPrice / length) + return (config.baseLength * config.maxPrice / length) / config.precisionMultiplier * config.precisionMultiplier; - - if(price < config.minPrice) return config.minPrice; - - return price; } /** diff --git a/test/ZNSCurvePricer.test.ts b/test/ZNSCurvePricer.test.ts index b45d88efb..d89af980d 100644 --- a/test/ZNSCurvePricer.test.ts +++ b/test/ZNSCurvePricer.test.ts @@ -301,7 +301,13 @@ describe("ZNSCurvePricer", () => { precisionMultiplier: precisionMultiDefault, feePercentage: registrationFeePercDefault, }; + // const tx = zns.curvePricer.connect(user).setPriceConfig(domainHash, newConfig); + + // const a = "abcdefghijabcdefghij" // 20 + // const b = "abcdefghijabcdefghijk" // 21 + // const price = await zns.curvePricer.getPrice(domainHash, a); + // console.log(price.toString()); await expect( zns.curvePricer.connect(user).setPriceConfig(domainHash, newConfig) ).to.be.revertedWith(CURVE_PRICE_CONFIG_ERR); From 61892ec851a5e77ac4c4c48fc240b98594176585 Mon Sep 17 00:00:00 2001 From: James Earle Date: Wed, 25 Oct 2023 17:00:16 -0400 Subject: [PATCH 4/6] remove comment --- test/ZNSCurvePricer.test.ts | 6 ------ 1 file changed, 6 deletions(-) diff --git a/test/ZNSCurvePricer.test.ts b/test/ZNSCurvePricer.test.ts index d89af980d..b45d88efb 100644 --- a/test/ZNSCurvePricer.test.ts +++ b/test/ZNSCurvePricer.test.ts @@ -301,13 +301,7 @@ describe("ZNSCurvePricer", () => { precisionMultiplier: precisionMultiDefault, feePercentage: registrationFeePercDefault, }; - // const tx = zns.curvePricer.connect(user).setPriceConfig(domainHash, newConfig); - - // const a = "abcdefghijabcdefghij" // 20 - // const b = "abcdefghijabcdefghijk" // 21 - // const price = await zns.curvePricer.getPrice(domainHash, a); - // console.log(price.toString()); await expect( zns.curvePricer.connect(user).setPriceConfig(domainHash, newConfig) ).to.be.revertedWith(CURVE_PRICE_CONFIG_ERR); From e40ba371a00cf736f1bccc8bcf419f17fde7d4bb Mon Sep 17 00:00:00 2001 From: James Earle Date: Thu, 26 Oct 2023 10:41:02 -0400 Subject: [PATCH 5/6] fix test to pass validation --- test/ZNSSubRegistrar.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/ZNSSubRegistrar.test.ts b/test/ZNSSubRegistrar.test.ts index 3c53b7c37..3c0d0aa32 100644 --- a/test/ZNSSubRegistrar.test.ts +++ b/test/ZNSSubRegistrar.test.ts @@ -1373,9 +1373,9 @@ describe("ZNSSubRegistrar", () => { expect(zeroVaultBalanceAfterRevoke.sub(zeroVaultBalanceAfter)).to.eq(0); }); - it("CurvePricer - StakePayment - stake fee - 13 decimals", async () => { + it.only("CurvePricer - StakePayment - stake fee - 13 decimals", async () => { const priceConfig = { - maxPrice: parseUnits("25000.93", decimalValues.thirteen), + maxPrice: parseUnits("30000.93", decimalValues.thirteen), minPrice: parseUnits("2000.11", decimalValues.thirteen), maxLength: BigNumber.from(50), baseLength: BigNumber.from(4), From aadc38092cd6695bc634c9e6775412f6367eabc3 Mon Sep 17 00:00:00 2001 From: James Earle Date: Thu, 26 Oct 2023 10:42:18 -0400 Subject: [PATCH 6/6] actually add change, remove .only --- test/ZNSSubRegistrar.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/ZNSSubRegistrar.test.ts b/test/ZNSSubRegistrar.test.ts index 3c0d0aa32..f45bf2cde 100644 --- a/test/ZNSSubRegistrar.test.ts +++ b/test/ZNSSubRegistrar.test.ts @@ -1373,7 +1373,7 @@ describe("ZNSSubRegistrar", () => { expect(zeroVaultBalanceAfterRevoke.sub(zeroVaultBalanceAfter)).to.eq(0); }); - it.only("CurvePricer - StakePayment - stake fee - 13 decimals", async () => { + it("CurvePricer - StakePayment - stake fee - 13 decimals", async () => { const priceConfig = { maxPrice: parseUnits("30000.93", decimalValues.thirteen), minPrice: parseUnits("2000.11", decimalValues.thirteen),