-
Notifications
You must be signed in to change notification settings - Fork 562
Cleanup, fix tests, config update #691
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -123,7 +123,7 @@ contract DropERC20Test_collectPrice is BaseTest { | |||||||||||||
uint256 afterBalancePlatformFeeRecipient = address(platformFeeRecipient).balance; | ||||||||||||||
uint256 defaultFeeRecipientAfter = address(defaultFeeRecipient).balance; | ||||||||||||||
|
||||||||||||||
uint256 defaultPlatformFeeVal = (pricePerToken * 100) / MAX_BPS; | ||||||||||||||
uint256 defaultPlatformFeeVal = (pricePerToken * 50) / MAX_BPS; | ||||||||||||||
uint256 platformFeeVal = (msgValue * platformFeeBps) / MAX_BPS; | ||||||||||||||
uint256 primarySaleRecipientVal = msgValue - platformFeeVal - defaultPlatformFeeVal; | ||||||||||||||
|
||||||||||||||
|
@@ -157,7 +157,7 @@ contract DropERC20Test_collectPrice is BaseTest { | |||||||||||||
uint256 defaultFeeRecipientAfter = erc20.balanceOf(defaultFeeRecipient); | ||||||||||||||
uint256 afterBalancePlatformFeeRecipient = erc20.balanceOf(platformFeeRecipient); | ||||||||||||||
|
||||||||||||||
uint256 defaultPlatformFeeVal = (pricePerToken * 100) / MAX_BPS; | ||||||||||||||
uint256 defaultPlatformFeeVal = (pricePerToken * 50) / MAX_BPS; | ||||||||||||||
uint256 platformFeeVal = (pricePerToken * platformFeeBps) / MAX_BPS; | ||||||||||||||
uint256 primarySaleRecipientVal = 1 ether - platformFeeVal - defaultPlatformFeeVal; | ||||||||||||||
|
||||||||||||||
Comment on lines
+160
to
163
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Use a named bps constant and avoid hardcoded 1 ether in proceeds calc
- uint256 defaultPlatformFeeVal = (pricePerToken * 50) / MAX_BPS;
+ uint256 defaultPlatformFeeVal = (pricePerToken * DEFAULT_FEE_BPS) / MAX_BPS;
uint256 platformFeeVal = (pricePerToken * platformFeeBps) / MAX_BPS;
- uint256 primarySaleRecipientVal = 1 ether - platformFeeVal - defaultPlatformFeeVal;
+ uint256 primarySaleRecipientVal = pricePerToken - platformFeeVal - defaultPlatformFeeVal; If you prefer a single source of truth, compute once: // outside changed lines
uint256 totalPrice = pricePerToken; // ERC20 branch
uint256 defaultPlatformFeeVal = (totalPrice * DEFAULT_FEE_BPS) / MAX_BPS;
uint256 platformFeeVal = (totalPrice * platformFeeBps) / MAX_BPS;
uint256 primarySaleRecipientVal = totalPrice - platformFeeVal - defaultPlatformFeeVal; 🤖 Prompt for AI Agents
|
||||||||||||||
|
@@ -187,7 +187,7 @@ contract DropERC20Test_collectPrice is BaseTest { | |||||||||||||
uint256 defaultFeeRecipientAfter = erc20.balanceOf(defaultFeeRecipient); | ||||||||||||||
uint256 afterBalancePlatformFeeRecipient = erc20.balanceOf(platformFeeRecipient); | ||||||||||||||
|
||||||||||||||
uint256 defaultPlatformFeeVal = (pricePerToken * 100) / MAX_BPS; | ||||||||||||||
uint256 defaultPlatformFeeVal = (pricePerToken * 50) / MAX_BPS; | ||||||||||||||
uint256 platformFeeVal = (pricePerToken * platformFeeBps) / MAX_BPS; | ||||||||||||||
uint256 primarySaleRecipientVal = 1 ether - platformFeeVal - defaultPlatformFeeVal; | ||||||||||||||
|
||||||||||||||
Comment on lines
+190
to
193
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Same improvement: constant bps and remove 1 ether literal Mirror the ERC20 proceeds calc fix to avoid hidden assumptions. - uint256 defaultPlatformFeeVal = (pricePerToken * 50) / MAX_BPS;
+ uint256 defaultPlatformFeeVal = (pricePerToken * DEFAULT_FEE_BPS) / MAX_BPS;
uint256 platformFeeVal = (pricePerToken * platformFeeBps) / MAX_BPS;
- uint256 primarySaleRecipientVal = 1 ether - platformFeeVal - defaultPlatformFeeVal;
+ uint256 primarySaleRecipientVal = pricePerToken - platformFeeVal - defaultPlatformFeeVal; 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents
|
||||||||||||||
|
@@ -216,7 +216,7 @@ contract DropERC20Test_collectPrice is BaseTest { | |||||||||||||
uint256 afterBalancePlatformFeeRecipient = address(platformFeeRecipient).balance; | ||||||||||||||
uint256 defaultFeeRecipientAfter = address(defaultFeeRecipient).balance; | ||||||||||||||
|
||||||||||||||
uint256 defaultPlatformFeeVal = (pricePerToken * 100) / MAX_BPS; | ||||||||||||||
uint256 defaultPlatformFeeVal = (pricePerToken * 50) / MAX_BPS; | ||||||||||||||
uint256 platformFeeVal = (msgValue * platformFeeBps) / MAX_BPS; | ||||||||||||||
uint256 primarySaleRecipientVal = msgValue - platformFeeVal - defaultPlatformFeeVal; | ||||||||||||||
|
||||||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -240,7 +240,7 @@ contract EnglishAuctionsPayoutTest is BaseTest, IExtension { | |
uint256 platformFees = (totalPrice * platformFeeBps) / 10_000; | ||
|
||
{ | ||
uint256 defaultFee = (totalPrice * 100) / 10_000; | ||
uint256 defaultFee = (totalPrice * 50) / 10_000; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Verification agent ❓ Verification inconclusiveAvoid magic numbers for default fee bps Use a named constant instead of hardcoding 50 to reduce brittleness and ease future updates. Apply within this test: - uint256 defaultFee = (totalPrice * 50) / 10_000;
+ uint256 defaultFee = (totalPrice * DEFAULT_FEE_BPS) / 10_000; Add near the top of the contract: uint256 private constant DEFAULT_FEE_BPS = 50; Optional: scan for any leftover 100 bps references still tied to default fee: 🏁 Script executed: #!/bin/bash
# Find potentially stale default-fee computations still using 100 bps in tests
rg -n --no-heading -e '\(\s*[^)]*\s*\*\s*100\s*\)\s*/\s*(MAX_BPS|10_000)' -e 'DEFAULT_FEE_BPS\s*=\s*100' src/test Length of output: 115 Define and use To eliminate the magic number and make future fee-bps updates easier, introduce a constant and reference it in your test: • At the top of uint256 private constant DEFAULT_FEE_BPS = 50; • Replace both occurrences of the hardcoded - uint256 defaultFee = (totalPrice * 50) / 10_000;
+ uint256 defaultFee = (totalPrice * DEFAULT_FEE_BPS) / 10_000; I ran a grep across 🤖 Prompt for AI Agents
|
||
|
||
// Platform fee recipient receives correct amount | ||
assertBalERC20Eq(address(erc20), platformFeeRecipient, platformFees); | ||
|
@@ -290,7 +290,7 @@ contract EnglishAuctionsPayoutTest is BaseTest, IExtension { | |
uint256 platformFees = (totalPrice * platformFeeBps) / 10_000; | ||
|
||
{ | ||
uint256 defaultFee = (totalPrice * 100) / 10_000; | ||
uint256 defaultFee = (totalPrice * 50) / 10_000; | ||
|
||
// Royalty recipients receive correct amounts | ||
assertBalERC20Eq(address(erc20), mockRecipients[0], mockAmounts[0]); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Default protocol fee reduced to 50 bps — verify downstream assumptions and update docs/tests
This halves the protocol take-rate (1% → 0.5%). Please confirm no tests, deployment configs, UI copy, or partner integrations assume 100 bps.
Run this to find assumptions tied to 100 bps / 1% and the default fee in the repo:
🏁 Script executed:
Length of output: 17309
Default protocol fee halved to 50 bps — update tests assuming 100 bps
Several marketplace tests hardcode a 1% (100 bps) default fee and must be revised to reflect the new 50 bps default:
• src/test/marketplace/Offers.t.sol:364
• src/test/marketplace/DirectListings.t.sol:590
• src/test/marketplace/EnglishAuctions.t.sol:425
• src/test/marketplace/english-auctions/_payout/_payout.t.sol:267
• src/test/marketplace/direct-listings/_payout/_payout.t.sol:292
Action items:
🤖 Prompt for AI Agents