Skip to content

Commit

Permalink
fix: precision issue (#493)
Browse files Browse the repository at this point in the history
  • Loading branch information
Mikelle authored Nov 21, 2024
1 parent 36254cd commit a529292
Show file tree
Hide file tree
Showing 20 changed files with 196 additions and 189 deletions.
18 changes: 9 additions & 9 deletions contracts-abi/abi/BidderRegistry.abi
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
},
{
"type": "function",
"name": "PERCENT",
"name": "ONE_HUNDRED_PERCENT",
"inputs": [],
"outputs": [
{
Expand Down Expand Up @@ -152,8 +152,8 @@
"outputs": [
{
"name": "",
"type": "uint16",
"internalType": "uint16"
"type": "uint256",
"internalType": "uint256"
}
],
"stateMutability": "view"
Expand Down Expand Up @@ -225,8 +225,8 @@
},
{
"name": "_feePercent",
"type": "uint16",
"internalType": "uint16"
"type": "uint256",
"internalType": "uint256"
},
{
"name": "_owner",
Expand Down Expand Up @@ -516,8 +516,8 @@
"inputs": [
{
"name": "newFeePercent",
"type": "uint16",
"internalType": "uint16"
"type": "uint256",
"internalType": "uint256"
}
],
"outputs": [],
Expand Down Expand Up @@ -755,9 +755,9 @@
"inputs": [
{
"name": "newFeePercent",
"type": "uint16",
"type": "uint256",
"indexed": true,
"internalType": "uint16"
"internalType": "uint256"
}
],
"anonymous": false
Expand Down
2 changes: 1 addition & 1 deletion contracts-abi/abi/Oracle.abi
Original file line number Diff line number Diff line change
Expand Up @@ -540,7 +540,7 @@
},
{
"type": "error",
"name": "ResidualBidPercentAfterDecayExceeds100",
"name": "ResidualBidPercentAfterDecayExceedsMax",
"inputs": [
{
"name": "residualBidPercentAfterDecay",
Expand Down
18 changes: 9 additions & 9 deletions contracts-abi/abi/ProviderRegistry.abi
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
},
{
"type": "function",
"name": "PERCENT",
"name": "ONE_HUNDRED_PERCENT",
"inputs": [],
"outputs": [
{
Expand Down Expand Up @@ -158,8 +158,8 @@
"outputs": [
{
"name": "",
"type": "uint16",
"internalType": "uint16"
"type": "uint256",
"internalType": "uint256"
}
],
"stateMutability": "view"
Expand Down Expand Up @@ -250,8 +250,8 @@
},
{
"name": "_feePercent",
"type": "uint16",
"internalType": "uint16"
"type": "uint256",
"internalType": "uint256"
},
{
"name": "_owner",
Expand Down Expand Up @@ -495,8 +495,8 @@
"inputs": [
{
"name": "newFeePercent",
"type": "uint16",
"internalType": "uint16"
"type": "uint256",
"internalType": "uint256"
}
],
"outputs": [],
Expand Down Expand Up @@ -705,9 +705,9 @@
"inputs": [
{
"name": "newFeePercent",
"type": "uint16",
"type": "uint256",
"indexed": true,
"internalType": "uint16"
"internalType": "uint256"
}
],
"anonymous": false
Expand Down
96 changes: 48 additions & 48 deletions contracts-abi/clients/BidderRegistry/BidderRegistry.go

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion contracts-abi/clients/Oracle/Oracle.go

Large diffs are not rendered by default.

96 changes: 48 additions & 48 deletions contracts-abi/clients/ProviderRegistry/ProviderRegistry.go

Large diffs are not rendered by default.

11 changes: 5 additions & 6 deletions contracts/contracts/core/BidderRegistry.sol
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ contract BidderRegistry is
*/
function initialize(
address _protocolFeeRecipient,
uint16 _feePercent,
uint256 _feePercent,
address _owner,
address _blockTracker,
uint256 _feePayoutPeriodBlocks
Expand Down Expand Up @@ -184,11 +184,10 @@ contract BidderRegistry is
require(bidState.state == State.PreConfirmed, BidNotPreConfirmed(commitmentDigest, bidState.state, State.PreConfirmed));

uint256 decayedAmt = (bidState.bidAmt *
residualBidPercentAfterDecay *
PRECISION) / PERCENT;
residualBidPercentAfterDecay) / ONE_HUNDRED_PERCENT;

uint256 feeAmt = (decayedAmt * uint256(feePercent) * PRECISION) /
PERCENT;
uint256 feeAmt = (decayedAmt * feePercent) /
ONE_HUNDRED_PERCENT;
uint256 amtMinusFeeAndDecay = decayedAmt - feeAmt;

protocolFeeTracker.accumulatedAmount += feeAmt;
Expand Down Expand Up @@ -306,7 +305,7 @@ contract BidderRegistry is
* @dev onlyOwner restriction
* @param newFeePercent this is the new fee percent
*/
function setNewFeePercent(uint16 newFeePercent) external onlyOwner {
function setNewFeePercent(uint256 newFeePercent) external onlyOwner {
feePercent = newFeePercent;
emit FeePercentUpdated(newFeePercent);
}
Expand Down
6 changes: 3 additions & 3 deletions contracts/contracts/core/BidderRegistryStorage.sol
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,14 @@ abstract contract BidderRegistryStorage {
using FeePayout for FeePayout.Tracker;

/// @dev For improved precision
uint256 constant public PRECISION = 1e25;
uint256 constant public PERCENT = 100 * PRECISION;
uint256 constant public PRECISION = 1e16;
uint256 constant public ONE_HUNDRED_PERCENT = 100 * PRECISION;

/// @dev Address of the preconfManager contract
address public preconfManager;

/// @dev Fee percent that would be taken by protocol when provider is slashed
uint16 public feePercent;
uint256 public feePercent;

/// @dev Block tracker contract
IBlockTracker public blockTrackerContract;
Expand Down
2 changes: 1 addition & 1 deletion contracts/contracts/core/Oracle.sol
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ contract Oracle is OracleStorage, IOracle, Ownable2StepUpgradeable, UUPSUpgradea
) external onlyOracle whenNotPaused {
address blockWinner = _blockTrackerContract.getBlockWinner(blockNumber);
require(blockWinner == builder, BuilderNotBlockWinner(blockWinner, builder));
require(residualBidPercentAfterDecay <= 100, ResidualBidPercentAfterDecayExceeds100(residualBidPercentAfterDecay));
require(residualBidPercentAfterDecay <= 1e18, ResidualBidPercentAfterDecayExceedsMax(residualBidPercentAfterDecay));

IPreconfManager.OpenedCommitment
memory commitment = _preconfManager.getCommitment(commitmentIndex);
Expand Down
8 changes: 4 additions & 4 deletions contracts/contracts/core/ProviderRegistry.sol
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ contract ProviderRegistry is
function initialize(
uint256 _minStake,
address _penaltyFeeRecipient,
uint16 _feePercent,
uint256 _feePercent,
address _owner,
uint256 _withdrawalDelay,
uint256 _penaltyFeePayoutPeriodBlocks
Expand Down Expand Up @@ -104,8 +104,8 @@ contract ProviderRegistry is
address payable bidder,
uint256 residualBidPercentAfterDecay
) external nonReentrant onlyPreconfManager whenNotPaused {
uint256 residualAmt = (amt * residualBidPercentAfterDecay * PRECISION) / PERCENT;
uint256 penaltyFee = (residualAmt * uint256(feePercent) * PRECISION) / PERCENT;
uint256 residualAmt = (amt * residualBidPercentAfterDecay) / ONE_HUNDRED_PERCENT;
uint256 penaltyFee = (residualAmt * feePercent) / ONE_HUNDRED_PERCENT;
uint256 providerStake = providerStakes[provider];

if (providerStake < residualAmt + penaltyFee) {
Expand Down Expand Up @@ -153,7 +153,7 @@ contract ProviderRegistry is
* @dev onlyOwner restriction
* @param newFeePercent this is the new fee percent
*/
function setNewFeePercent(uint16 newFeePercent) external onlyOwner {
function setNewFeePercent(uint256 newFeePercent) external onlyOwner {
feePercent = newFeePercent;
emit FeePercentUpdated(newFeePercent);
}
Expand Down
6 changes: 3 additions & 3 deletions contracts/contracts/core/ProviderRegistryStorage.sol
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ abstract contract ProviderRegistryStorage {
using FeePayout for FeePayout.Tracker;

/// @dev For improved precision
uint256 public constant PRECISION = 1e25;
uint256 public constant PERCENT = 100 * PRECISION;
uint256 public constant PRECISION = 1e16;
uint256 public constant ONE_HUNDRED_PERCENT = 100 * PRECISION;

/// @dev Minimum stake required for registration
uint256 public minStake;
Expand All @@ -17,7 +17,7 @@ abstract contract ProviderRegistryStorage {
address public preconfManager;

/// @dev Fee percent that would be taken by protocol when provider is slashed
uint16 public feePercent;
uint256 public feePercent;

/// @dev Configurable withdrawal delay in milliseconds
uint256 public withdrawalDelay;
Expand Down
2 changes: 1 addition & 1 deletion contracts/contracts/interfaces/IBidderRegistry.sol
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ interface IBidderRegistry {
event PreconfManagerUpdated(address indexed newPreconfManager);

/// @dev Event emitted when the fee percent is updated
event FeePercentUpdated(uint16 indexed newFeePercent);
event FeePercentUpdated(uint256 indexed newFeePercent);

/// @dev Event emitted when the block tracker is updated
event BlockTrackerUpdated(address indexed newBlockTracker);
Expand Down
4 changes: 2 additions & 2 deletions contracts/contracts/interfaces/IOracle.sol
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ interface IOracle {
/// @dev Error emitted when the builder is not the block winner
error BuilderNotBlockWinner(address blockWinner, address builder);

/// @dev Error emitted when the residual bid percent after decay exceeds 100
error ResidualBidPercentAfterDecayExceeds100(uint256 residualBidPercentAfterDecay);
/// @dev Error emitted when the residual bid percent after decay exceeds max amount
error ResidualBidPercentAfterDecayExceedsMax(uint256 residualBidPercentAfterDecay);

receive() external payable;

Expand Down
2 changes: 1 addition & 1 deletion contracts/contracts/interfaces/IProviderRegistry.sol
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ interface IProviderRegistry {
event PreconfManagerUpdated(address indexed newPreconfManager);

/// @dev Event emitted when the fee percent is updated
event FeePercentUpdated(uint16 indexed newFeePercent);
event FeePercentUpdated(uint256 indexed newFeePercent);

/// @dev Event emitted when there are insufficient funds to slash
event InsufficientFundsToSlash(
Expand Down
7 changes: 4 additions & 3 deletions contracts/scripts/core/DeployCore.s.sol
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@ contract DeployTestnet is Script {

// Amount of ETH to initially fund the oracle account on L1 chain.
uint256 public constant ORACLE_INITIAL_FUNDING = 1 ether;

uint256 public constant PERCENT_MULTIPLIER = 1e16;

error FailedToSendETHToOracle(address addr);

function run() external {
Expand All @@ -29,8 +30,8 @@ contract DeployTestnet is Script {
address protocolFeeRecipient = address(
0xfA0B0f5d298d28EFE4d35641724141ef19C05684 // Placeholder for now, L1 preconf.eth address
);
uint16 feePercent = 2;
uint16 providerPenaltyPercent = 5;
uint256 feePercent = 2 * PERCENT_MULTIPLIER; // 2%
uint256 providerPenaltyPercent = 5 * PERCENT_MULTIPLIER; // 5%
uint64 commitmentDispatchWindow = 2000;
uint256 withdrawalDelay = 24 hours * 1000; // 24 hours in milliseconds
uint256 protocolFeePayoutPeriodBlocks = 5 hours ; // 1 hour with 200ms blocks
Expand Down
18 changes: 9 additions & 9 deletions contracts/test/core/BidderRegistryTest.sol
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import {ProviderRegistry} from "../../contracts/core/ProviderRegistry.sol";
contract BidderRegistryTest is Test {
uint256 public testNumber;
BidderRegistry public bidderRegistry;
uint16 public feePercent;
uint256 public feePercent;
uint256 public minStake;
address public bidder;
address public feeRecipient;
Expand All @@ -29,7 +29,7 @@ contract BidderRegistryTest is Test {

function setUp() public {
testNumber = 42;
feePercent = 10;
feePercent = 10 * 1e16;
minStake = 1e18 wei;
feeRecipient = vm.addr(9);
feePayoutPeriodBlocks = 100;
Expand Down Expand Up @@ -194,7 +194,7 @@ contract BidderRegistryTest is Test {

bidderRegistry.openBid(commitmentDigest, 1 ether, bidder, blockNumber);

bidderRegistry.retrieveFunds(nextWindow, commitmentDigest, payable(provider), 100);
bidderRegistry.retrieveFunds(nextWindow, commitmentDigest, payable(provider), bidderRegistry.ONE_HUNDRED_PERCENT());
uint256 providerAmount = bidderRegistry.providerAmount(provider);
uint256 feeRecipientAmount = bidderRegistry.getAccumulatedProtocolFee();

Expand All @@ -220,7 +220,7 @@ contract BidderRegistryTest is Test {

uint256 bidderBalance = bidder.balance;

bidderRegistry.retrieveFunds(nextWindow, commitmentDigest, payable(provider), 50);
bidderRegistry.retrieveFunds(nextWindow, commitmentDigest, payable(provider), 50 * bidderRegistry.PRECISION());
uint256 providerAmount = bidderRegistry.providerAmount(provider);
uint256 feeRecipientAmount = bidderRegistry.getAccumulatedProtocolFee();

Expand Down Expand Up @@ -269,7 +269,7 @@ contract BidderRegistryTest is Test {
vm.expectRevert(bytes(""));
bytes32 commitmentDigest = keccak256("1234");
bidderRegistry.openBid(commitmentDigest, 1 ether, bidder, blockNumber);
bidderRegistry.retrieveFunds(nextWindow, commitmentDigest, payable(provider),100);
bidderRegistry.retrieveFunds(nextWindow, commitmentDigest, payable(provider), bidderRegistry.ONE_HUNDRED_PERCENT());
}

function testFail_shouldRetrieveFundsGreaterThanStake() public {
Expand All @@ -287,7 +287,7 @@ contract BidderRegistryTest is Test {
vm.prank(address(this));
bytes32 commitmentDigest = keccak256("1234");
bidderRegistry.openBid(commitmentDigest, 3 ether, bidder, blockNumber);
bidderRegistry.retrieveFunds(nextWindow, commitmentDigest, payable(provider),100);
bidderRegistry.retrieveFunds(nextWindow, commitmentDigest, payable(provider), bidderRegistry.ONE_HUNDRED_PERCENT());
}

function test_withdrawProviderAmount() public {
Expand All @@ -305,7 +305,7 @@ contract BidderRegistryTest is Test {

bidderRegistry.openBid(commitmentDigest, 2 ether, bidder, blockNumber);

bidderRegistry.retrieveFunds(nextWindow, commitmentDigest, payable(provider), 100);
bidderRegistry.retrieveFunds(nextWindow, commitmentDigest, payable(provider), bidderRegistry.ONE_HUNDRED_PERCENT());
bidderRegistry.withdrawProviderAmount(payable(provider));
uint256 balanceAfter = address(provider).balance;
assertEq(balanceAfter - balanceBefore, 1800000000000000000);
Expand Down Expand Up @@ -451,7 +451,7 @@ contract BidderRegistryTest is Test {
bidderRegistry.openBid(commitmentDigest, 1 ether, bidder, blockNumber);
vm.expectEmit(true, true, true, true);
emit FeeTransfer(100000000000000000, feeRecipient);
bidderRegistry.retrieveFunds(nextWindow, commitmentDigest, payable(provider),100);
bidderRegistry.retrieveFunds(nextWindow, commitmentDigest, payable(provider), bidderRegistry.ONE_HUNDRED_PERCENT());
uint256 balanceAfter = feeRecipient.balance;
assertEq(balanceAfter - balanceBefore, 100000000000000000);
assertEq(bidderRegistry.getAccumulatedProtocolFee(), 0);
Expand All @@ -470,7 +470,7 @@ contract BidderRegistryTest is Test {
blockTracker.addBuilderAddress("test", provider);
blockTracker.recordL1Block(blockNumber, "test");
bidderRegistry.openBid(commitmentDigest, 1 ether, bidder, blockNumber);
bidderRegistry.retrieveFunds(nextWindow, commitmentDigest, payable(provider),100);
bidderRegistry.retrieveFunds(nextWindow, commitmentDigest, payable(provider), bidderRegistry.ONE_HUNDRED_PERCENT());
uint256 balanceAfter = feeRecipient.balance;
assertEq(balanceAfter - balanceBefore, 0);
assertEq(bidderRegistry.getAccumulatedProtocolFee(), 100000000000000000);
Expand Down
Loading

0 comments on commit a529292

Please sign in to comment.