Skip to content
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.

Sign up for GitHub

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

burn network fees #489

Merged
merged 5 commits into from
Oct 26, 2023
Merged

burn network fees #489

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d81f294
Merge pull request #473 from bancorprotocol/dev
yudilevi Mar 16, 2023
0fe72c9
Merge pull request #481 from bancorprotocol/dev
yudilevi Jul 14, 2023
3b566da
Merge pull request #488 from bancorprotocol/dev
yudilevi Sep 13, 2023
bb50fd7
burn network fees
ivanzhelyazkov Oct 19, 2023
094af71
add upgrade scripts
ivanzhelyazkov Oct 20, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 47 additions & 29 deletions contracts/network/BancorNetwork.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,9 +95,6 @@ contract BancorNetwork is IBancorNetwork, Upgradeable, ReentrancyGuardUpgradeabl
// the emergency manager role is required to pause/unpause the network
bytes32 private constant ROLE_EMERGENCY_STOPPER = keccak256("ROLE_EMERGENCY_STOPPER");

// the network fee manager role is required to pull the accumulated pending network fees
bytes32 private constant ROLE_NETWORK_FEE_MANAGER = keccak256("ROLE_NETWORK_FEE_MANAGER");

// the address of the BNT token
IERC20 private immutable _bnt;

Expand Down Expand Up @@ -149,15 +146,18 @@ contract BancorNetwork is IBancorNetwork, Upgradeable, ReentrancyGuardUpgradeabl
// a mapping between pools and their respective pool collections
mapping(Token => IPoolCollection) private _collectionByPool;

// the pending network fee amount to be burned by the vortex
// the pending network fee amount to be burned
uint256 internal _pendingNetworkFeeAmount;

bool private _depositingEnabled = true;

uint32 private _polRewardsPPM;

// min network fee amount that can be burned
uint256 private _minNetworkFeeBurn;
lbeder marked this conversation as resolved.
Show resolved Hide resolved

// upgrade forward-compatibility storage gap
uint256[MAX_GAP - 11] private __gap;
uint256[MAX_GAP - 12] private __gap;

/**
* @dev triggered when a new pool collection is added
Expand Down Expand Up @@ -217,9 +217,9 @@ contract BancorNetwork is IBancorNetwork, Upgradeable, ReentrancyGuardUpgradeabl
event FlashLoanCompleted(Token indexed token, address indexed borrower, uint256 amount, uint256 feeAmount);

/**
* @dev triggered when network fees are withdrawn
* @dev triggered when network fees are burned
*/
event NetworkFeesWithdrawn(address indexed caller, address indexed recipient, uint256 amount);
event NetworkFeesBurned(address indexed caller, uint256 amount);

/**
* @dev triggered when pool surplus tokens are withdrawn
Expand All @@ -231,6 +231,11 @@ contract BancorNetwork is IBancorNetwork, Upgradeable, ReentrancyGuardUpgradeabl
*/
event POLRewardsPPMUpdated(uint32 oldRewardsPPM, uint32 newRewardsPPM);

/**
* @dev triggered when the min network fee burn is updated
*/
event MinNetworkFeeBurnUpdated(uint256 oldMinNetworkFeeBurn, uint256 newMinNetworkFeeBurn);

/**
* @dev a "virtual" constructor that is only used to set immutable state variables
*/
Expand Down Expand Up @@ -315,11 +320,11 @@ contract BancorNetwork is IBancorNetwork, Upgradeable, ReentrancyGuardUpgradeabl
// set up administrative roles
_setRoleAdmin(ROLE_MIGRATION_MANAGER, ROLE_ADMIN);
_setRoleAdmin(ROLE_EMERGENCY_STOPPER, ROLE_ADMIN);
_setRoleAdmin(ROLE_NETWORK_FEE_MANAGER, ROLE_ADMIN);

_depositingEnabled = true;

_setPOLRewardsPPM(2000);
_setMinNetworkFeeBurn(1_000_000e18);
lbeder marked this conversation as resolved.
Show resolved Hide resolved
}

// solhint-enable func-name-mixedcase
Expand Down Expand Up @@ -360,14 +365,7 @@ contract BancorNetwork is IBancorNetwork, Upgradeable, ReentrancyGuardUpgradeabl
}

/**
* @dev returns the network fee manager role
*/
function roleNetworkFeeManager() external pure returns (bytes32) {
return ROLE_NETWORK_FEE_MANAGER;
}

/**
* @dev returns the pending network fee amount to be burned by the vortex
* @dev returns the pending network fee amount to be burned
*/
function pendingNetworkFeeAmount() external view returns (uint256) {
return _pendingNetworkFeeAmount;
Expand Down Expand Up @@ -820,26 +818,17 @@ contract BancorNetwork is IBancorNetwork, Upgradeable, ReentrancyGuardUpgradeabl
/**
* @inheritdoc IBancorNetwork
*/
function withdrawNetworkFees(
address recipient
)
external
whenNotPaused
onlyRoleMember(ROLE_NETWORK_FEE_MANAGER)
validAddress(recipient)
nonReentrant
returns (uint256)
{
function burnNetworkFees() external whenNotPaused nonReentrant returns (uint256) {
uint256 currentPendingNetworkFeeAmount = _pendingNetworkFeeAmount;
if (currentPendingNetworkFeeAmount == 0) {
if (currentPendingNetworkFeeAmount < _minNetworkFeeBurn) {
return 0;
}

_pendingNetworkFeeAmount = 0;

_masterVault.withdrawFunds(Token(address(_bnt)), payable(recipient), currentPendingNetworkFeeAmount);
_masterVault.withdrawFunds(Token(address(_bnt)), payable(address(_bnt)), currentPendingNetworkFeeAmount);
lbeder marked this conversation as resolved.
Show resolved Hide resolved

emit NetworkFeesWithdrawn(msg.sender, recipient, currentPendingNetworkFeeAmount);
emit NetworkFeesBurned(msg.sender, currentPendingNetworkFeeAmount);

return currentPendingNetworkFeeAmount;
}
Expand Down Expand Up @@ -915,6 +904,35 @@ contract BancorNetwork is IBancorNetwork, Upgradeable, ReentrancyGuardUpgradeabl
emit POLRewardsPPMUpdated(oldRewardsPPM, newRewardsPPM);
}

/**
* @dev returns the min network fee burn
*/
function minNetworkFeeBurn() external view returns (uint256) {
return _minNetworkFeeBurn;
}

/**
* @dev set the min network fee burn
*/
function setMinNetworkFeeBurn(
uint256 newMinNetworkFeeBurn
) external onlyAdmin greaterThanZero(newMinNetworkFeeBurn) {
_setMinNetworkFeeBurn(newMinNetworkFeeBurn);
}

/**
* @dev set the min network fee burn
*/
function _setMinNetworkFeeBurn(uint256 newMinNetworkFeeBurn) private {
uint256 oldMinNetworkFeeBurn = _minNetworkFeeBurn;
if (oldMinNetworkFeeBurn == newMinNetworkFeeBurn) {
return;
}

_minNetworkFeeBurn = newMinNetworkFeeBurn;
emit MinNetworkFeeBurnUpdated(oldMinNetworkFeeBurn, newMinNetworkFeeBurn);
}

/**
* @dev generates context ID for a deposit request
*/
Expand Down
8 changes: 2 additions & 6 deletions contracts/network/interfaces/IBancorNetwork.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -218,13 +218,9 @@ interface IBancorNetwork is IUpgradeable {
) external payable;

/**
* @dev withdraws pending network fees, and returns the amount of fees withdrawn
*
* requirements:
*
* - the caller must have the ROLE_NETWORK_FEE_MANAGER privilege
* @dev burns pending network fees, and returns the amount of fees burned
*/
function withdrawNetworkFees(address recipient) external returns (uint256);
function burnNetworkFees() external returns (uint256);

/**
* @dev withdraws surplus tokens from a given pool to CarbonPOL contract,
Expand Down
1 change: 0 additions & 1 deletion deploy/tests/network.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,6 @@ import { getNamedAccounts } from 'hardhat';
await expectRoleMembers(network, Roles.Upgradeable.ROLE_ADMIN, [daoMultisig.address]);
await expectRoleMembers(network, Roles.BancorNetwork.ROLE_MIGRATION_MANAGER, [liquidityProtection.address]);
await expectRoleMembers(network, Roles.BancorNetwork.ROLE_EMERGENCY_STOPPER);
await expectRoleMembers(network, Roles.BancorNetwork.ROLE_NETWORK_FEE_MANAGER, [daoMultisig.address]);

await expectRoleMembers(standardRewards, Roles.Upgradeable.ROLE_ADMIN, [daoMultisig.address]);

Expand Down
112 changes: 74 additions & 38 deletions test/network/BancorNetwork.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,6 @@ describe('BancorNetwork', () => {
await expectRole(network, Roles.Upgradeable.ROLE_ADMIN, Roles.Upgradeable.ROLE_ADMIN, [deployer.address]);
await expectRole(network, Roles.BancorNetwork.ROLE_MIGRATION_MANAGER, Roles.Upgradeable.ROLE_ADMIN);
await expectRole(network, Roles.BancorNetwork.ROLE_EMERGENCY_STOPPER, Roles.Upgradeable.ROLE_ADMIN);
await expectRole(network, Roles.BancorNetwork.ROLE_NETWORK_FEE_MANAGER, Roles.Upgradeable.ROLE_ADMIN);

expect(await network.paused()).to.be.false;
expect(await network.poolCollections()).to.be.empty;
Expand Down Expand Up @@ -3224,29 +3223,53 @@ describe('BancorNetwork', () => {
await network
.connect(deployer)
.grantRole(Roles.BancorNetwork.ROLE_EMERGENCY_STOPPER, emergencyStopper.address);
await network
.connect(deployer)
.grantRole(Roles.BancorNetwork.ROLE_NETWORK_FEE_MANAGER, networkFeeManager.address);
});

it('should revert when a non-network fee manager is attempting to withdraw the fees', async () => {
await expect(network.connect(deployer).pause()).to.be.revertedWithError('AccessDenied');
it('should revert when a non-admin is attempting to set the min burn amount', async () => {
await expect(network.connect(emergencyStopper).setMinNetworkFeeBurn(1)).to.be.revertedWithError(
'AccessDenied'
);
});

it('should revert when attempting to set min burn amount to an invalid amount', async () => {
await expect(network.setMinNetworkFeeBurn(0)).to.be.revertedWithError('ZeroValue');
});

it('admin should be able to set min network fee burn', async () => {
const newMinNetworkFeeBurn = toWei(1000);
await network.connect(deployer).setMinNetworkFeeBurn(newMinNetworkFeeBurn);
const minNetworkFeeBurn = await network.minNetworkFeeBurn();
expect(minNetworkFeeBurn).to.be.eq(newMinNetworkFeeBurn);
});

it('setting min network fee burn should emit an event', async () => {
const newMinNetworkFeeBurn = toWei(1000);
const oldNetworkFeeBurn = await network.minNetworkFeeBurn();
await expect(network.connect(deployer).setMinNetworkFeeBurn(newMinNetworkFeeBurn))
.to.emit(network, 'MinNetworkFeeBurnUpdated')
.withArgs(oldNetworkFeeBurn, newMinNetworkFeeBurn);
});

it('should ignore setting the min network fee burn to the same value', async () => {
const oldMinNetworkFeeBurn = await network.minNetworkFeeBurn();
await expect(network.connect(deployer).setMinNetworkFeeBurn(oldMinNetworkFeeBurn)).not.to.emit(
network,
'MinNetworkFeeBurnUpdated'
);
});

context('without any pending network fees', () => {
it('should not withdraw any pending network fees', async () => {
const prevBNTBalance = await bnt.balanceOf(networkFeeManager.address);
it('should not burn any pending network fees', async () => {
const burnedNetworkFees = await network.callStatic.burnNetworkFees();
expect(burnedNetworkFees).to.equal(0);

const withdrawNetworkFees = await network
.connect(networkFeeManager)
.callStatic.withdrawNetworkFees(networkFeeManager.address);
expect(withdrawNetworkFees).to.equal(0);
const totalSupplyBefore = await bnt.totalSupply();

const res = await network.connect(networkFeeManager).withdrawNetworkFees(networkFeeManager.address);
const res = await network.burnNetworkFees();

await expect(res).to.not.emit(network, 'NetworkFeesWithdrawn');
await expect(res).to.not.emit(network, 'NetworkFeesBurned');

expect(await bnt.balanceOf(networkFeeManager.address)).to.equal(prevBNTBalance);
expect(await bnt.totalSupply()).to.equal(totalSupplyBefore);
});
});

Expand All @@ -3255,37 +3278,52 @@ describe('BancorNetwork', () => {
await tradeBySourceAmount(deployer, bnt, token, toWei(1000), 1, MAX_UINT256, deployer.address, network);

expect(await network.pendingNetworkFeeAmount()).to.be.gt(0);
expect(await network.minNetworkFeeBurn()).to.be.eq(toWei(1_000_000));
});

it('should revert when the withdrawal caller is not a network-fee manager', async () => {
await expect(
network.connect(deployer).withdrawNetworkFees(networkFeeManager.address)
).to.be.revertedWithError('AccessDenied');
});
it('should not burn any pending network fees if the pending amount is below the min network fee burn amount', async () => {
const minNetworkFeeBurnAmount = await network.minNetworkFeeBurn();
expect(await network.pendingNetworkFeeAmount()).to.be.lt(minNetworkFeeBurnAmount);

it('should revert when the withdrawal recipient is invalid', async () => {
await expect(
network.connect(networkFeeManager).withdrawNetworkFees(ZERO_ADDRESS)
).to.be.revertedWithError('InvalidAddress');
const burnedNetworkFees = await network.callStatic.burnNetworkFees();
expect(burnedNetworkFees).to.equal(0);

const totalSupplyBefore = await bnt.totalSupply();

const res = await network.burnNetworkFees();

await expect(res).to.not.emit(network, 'NetworkFeesBurned');

expect(await bnt.totalSupply()).to.equal(totalSupplyBefore);
});

it('should withdraw all the pending network fees', async () => {
const recipient = nonOwner.address;
const prevBNTBalance = await bnt.balanceOf(networkFeeManager.address);
it('should burn all the pending network fees if the amount is above the min network fee burn amount', async () => {
await tradeBySourceAmount(
deployer,
bnt,
token,
toWei(10000000),
1,
MAX_UINT256,
deployer.address,
network
);
const minNetworkFeeBurnAmount = await network.minNetworkFeeBurn();
const pendingNetworkFeeAmount = await network.pendingNetworkFeeAmount();
expect(await network.pendingNetworkFeeAmount()).to.be.gt(minNetworkFeeBurnAmount);

const burnedNetworkFees = await network.callStatic.burnNetworkFees();
expect(burnedNetworkFees).to.equal(pendingNetworkFeeAmount);

const withdrawNetworkFees = await network
.connect(networkFeeManager)
.callStatic.withdrawNetworkFees(recipient);
expect(withdrawNetworkFees).to.equal(pendingNetworkFeeAmount);
const totalSupplyBefore = await bnt.totalSupply();

const res = await network.connect(networkFeeManager).withdrawNetworkFees(recipient);
const res = await network.burnNetworkFees();

await expect(res)
.to.emit(network, 'NetworkFeesWithdrawn')
.withArgs(networkFeeManager.address, recipient, pendingNetworkFeeAmount);
.to.emit(network, 'NetworkFeesBurned')
.withArgs(deployer.address, pendingNetworkFeeAmount);

expect(await bnt.balanceOf(recipient)).to.equal(prevBNTBalance.add(pendingNetworkFeeAmount));
expect(await bnt.totalSupply()).to.equal(totalSupplyBefore.sub(pendingNetworkFeeAmount));

expect(await network.pendingNetworkFeeAmount()).to.equal(0);
});
Expand All @@ -3296,9 +3334,7 @@ describe('BancorNetwork', () => {
});

it('should revert when attempting to withdraw the pending network fees', async () => {
await expect(
network.connect(networkFeeManager).withdrawNetworkFees(networkFeeManager.address)
).to.be.revertedWithError('Pausable: paused');
await expect(network.burnNetworkFees()).to.be.revertedWithError('Pausable: paused');
});
});
});
Expand Down
3 changes: 1 addition & 2 deletions utils/Roles.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,7 @@ export const Roles = {

BancorNetwork: {
ROLE_MIGRATION_MANAGER: id('ROLE_MIGRATION_MANAGER'),
ROLE_EMERGENCY_STOPPER: id('ROLE_EMERGENCY_STOPPER'),
ROLE_NETWORK_FEE_MANAGER: id('ROLE_NETWORK_FEE_MANAGER')
ROLE_EMERGENCY_STOPPER: id('ROLE_EMERGENCY_STOPPER')
},

MasterVault: {
Expand Down
Loading