Skip to content

SOV-4106 Feat: Fee sharing collector with direct transfer #2

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

Open
cwsnt wants to merge 5 commits into
base: bobDevelopment
Choose a base branch
from
Open

SOV-4106 Feat: Fee sharing collector with direct transfer #2

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ce99bf0
feat: support fee sharing collector
May 24, 2024
9f4da78
init: convertion lib dex
Jun 4, 2024
187c965
add readme for on going lib
Jun 4, 2024
97b3b40
init: collect protocol fee with direct transfer
Jun 12, 2024
aa9b3d4
1/ address comment
Jul 10, 2024
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
2 changes: 1 addition & 1 deletion contracts/SdexEvents.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ library SdexEvents {
/* @notice Emitted when accumulated protocol fees are collected by the treasury.
* @param token The token of the fees being collected.
* @param recv The vault the collected fees are being paid to. */
event ProtocolDividend (address indexed token, address indexed recv);
event ProtocolDividend (address[] indexed tokens, address indexed recv);

/* @notice Called when any proxy sidecar contract is upgraded.
* @param proxy The address of the new proxy smart contract.
Expand Down
27 changes: 18 additions & 9 deletions contracts/callpaths/ColdPath.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,15 @@ contract ColdPath is MarketSequencer, DepositDesk, ProtocolAccount {
using Chaining for Chaining.PairFlow;
using ProtocolCmd for bytes;

/** @dev ONLY APPLY CONSTANT VARIABLE HERE */
uint256 public constant TREASURY_START_TIME_OFFSET = 0 days;

/* @dev access control for treasury role */
modifier onlyTreasury() {
require(msg.sender == treasury_, "Only Treasury");
_;
}

/* @notice Consolidated method for protocol control related commands. */
function protocolCmd (bytes calldata cmd) virtual public {
uint8 code = uint8(cmd[31]);
Expand Down Expand Up @@ -71,9 +80,7 @@ contract ColdPath is MarketSequencer, DepositDesk, ProtocolAccount {
require(sudoMode_, "Sudo");
uint8 cmdCode = uint8(cmd[31]);

if (cmdCode == ProtocolCmd.COLLECT_TREASURY_CODE) {
collectProtocol(cmd);
} else if (cmdCode == ProtocolCmd.SET_TREASURY_CODE) {
if (cmdCode == ProtocolCmd.SET_TREASURY_CODE) {
setTreasury(cmd);
} else if (cmdCode == ProtocolCmd.AUTHORITY_TRANSFER_CODE) {
transferAuthority(cmd);
Expand Down Expand Up @@ -110,6 +117,8 @@ contract ColdPath is MarketSequencer, DepositDesk, ProtocolAccount {
resetNonceCond(cmd);
} else if (cmdCode == UserCmd.GATE_ORACLE_COND) {
checkGateOracle(cmd);
} else if (cmdCode == UserCmd.COLLECT_TREASURY_CODE) {
collectProtocol(cmd);
} else {
revert("Invalid command");
}
Expand Down Expand Up @@ -242,22 +251,22 @@ contract ColdPath is MarketSequencer, DepositDesk, ProtocolAccount {
/* @notice Pays out the the protocol fees.
* @param token The token for which the accumulated fees are being paid out.
* (Or if 0x0 pays out native Ethereum.) */
function collectProtocol (bytes calldata cmd) private {
(, address token) = abi.decode(cmd, (uint8, address));
function collectProtocol (bytes calldata cmd) private onlyTreasury {
(, address[] memory tokens) = abi.decode(cmd, (uint8, address[]));

require(block.timestamp >= treasuryStartTime_, "Treasury start");
emit SdexEvents.ProtocolDividend(token, treasury_);
disburseProtocolFees(treasury_, token);
emit SdexEvents.ProtocolDividend(tokens, treasury_);
disburseProtocolFees(tokens);
}

/* @notice Sets the treasury address to receive protocol fees. Once set, the treasury cannot
* receive fees until 7 days after. */
* receive fees until the start time offset. */
function setTreasury (bytes calldata cmd) private {
(, address treasury) = abi.decode(cmd, (uint8, address));

require(treasury != address(0) && treasury.code.length != 0, "Treasury invalid");
treasury_ = treasury;
Copy link
Contributor

@tjcloa tjcloa Jun 26, 2024

Choose a reason for hiding this comment

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

to keep it universal i suggest to restore the original code

treasuryStartTime_ = uint64(block.timestamp + 7 days);

but replace 7 days with a constant TREASURY_START_TIME_OFFSET to have this option if needed on other chains.
The BOB and default implementation should have it set to 0.

treasuryStartTime_ = uint64(block.timestamp + 7 days);
treasuryStartTime_ = uint64(block.timestamp + TREASURY_START_TIME_OFFSET);
emit SdexEvents.TreasurySet(treasury_, treasuryStartTime_);
}

Expand Down
3 changes: 1 addition & 2 deletions contracts/callpaths/WarmPath.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import '../mixins/MarketSequencer.sol';
import '../mixins/SettleLayer.sol';
import '../mixins/PoolRegistry.sol';
import '../mixins/MarketSequencer.sol';
import '../mixins/ProtocolAccount.sol';
import '../SdexEvents.sol';

/* @title Warm path callpath sidecar.
Expand All @@ -31,7 +30,7 @@ import '../SdexEvents.sol';
* not state. As such it should never be called directly or externally, and should
* only be invoked with DELEGATECALL so that it operates on the contract state
* within the primary SdexSwap contract. */
contract WarmPath is MarketSequencer, SettleLayer, ProtocolAccount {
contract WarmPath is MarketSequencer, SettleLayer {

using SafeCast for uint128;
using TokenFlow for TokenFlow.PairSeq;
Expand Down
8 changes: 8 additions & 0 deletions contracts/interfaces/IFeeProtocolCollector.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
// SPDX-License-Identifier: GPL-2.0-or-later
pragma solidity 0.8.19;

/// @title Minimal ERC20 interface for Uniswap
/// @notice Contains a subset of the full ERC20 interface that is used in Uniswap V3
interface IFeeProtocolCollector {
function transferTokens(address _token, uint96 _amount) external;
}
9 changes: 7 additions & 2 deletions contracts/libraries/ProtocolCmd.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,6 @@ library ProtocolCmd {
uint8 constant HOT_OPEN_CODE = 22;
// Code to toggle on or off emergency safe mode
uint8 constant SAFE_MODE_CODE = 23;
// Code to collect accumulated protocol fees for the treasury.
uint8 constant COLLECT_TREASURY_CODE = 40;
// Code to set the protocol treasury
uint8 constant SET_TREASURY_CODE = 41;
////////////////////////////////////////////////////////////////////////////
Expand Down Expand Up @@ -113,4 +111,11 @@ library UserCmd {
uint8 constant BURN_KNOCKOUT = 92;
uint8 constant CLAIM_KNOCKOUT = 93;
uint8 constant RECOVER_KNOCKOUT = 94;


////////////////////////////////////////////////////////////////////////////
// Protocol Fee command codes
////////////////////////////////////////////////////////////////////////////
// Code to collect accumulated protocol fees for the treasury.
uint8 constant COLLECT_TREASURY_CODE = 40;
}
34 changes: 26 additions & 8 deletions contracts/mixins/ProtocolAccount.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ import '../libraries/TokenFlow.sol';
import '../libraries/SafeCast.sol';
import './StorageLayout.sol';

import '../interfaces/IFeeProtocolCollector.sol';

/* @title Protocol Account Mixin
Copy link
Contributor

@tjcloa tjcloa Jun 26, 2024

Choose a reason for hiding this comment

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

the ProtocolAccount contract is inherited in WarmPath, but seems not to be used there.
pls double check and remove the inheritance.
the contract doesn't have it own storage, so should be safe.

* @notice Tracks and pays out the accumulated protocol fees across the entire exchange
* These are the fees belonging to the SdexSwap protocol, not the liquidity
Expand All @@ -16,6 +18,9 @@ import './StorageLayout.sol';
contract ProtocolAccount is StorageLayout {
using SafeCast for uint256;
using TokenFlow for address;

address public constant PROTOCOL_FEES_RECEIVER_HASH =
address(uint160(uint256(keccak256("PROTOCOL_FEES_RECEIVER_HASH"))));

/* @notice Called at the completion of a swap event, incrementing any protocol
* fees accumulated in the swap. */
Expand All @@ -35,14 +40,27 @@ contract ProtocolAccount is StorageLayout {
}

/* @notice Pays out the earned, but unclaimed protocol fees in the pool.
* @param recv - The receiver of the protocol fees.
* @param token - The token address of the quote token. */
function disburseProtocolFees (address recv, address token) internal {
uint128 collected = feesAccum_[token];
feesAccum_[token] = 0;
if (collected > 0) {
bytes32 payoutKey = keccak256(abi.encode(recv, token));
userBals_[payoutKey].surplusCollateral_ += collected;
* @param tokens - The token address of the quote token.
*/
function disburseProtocolFees (address[] memory tokens) internal {
for(uint256 i = 0; i < tokens.length; i++) {
address token = tokens[i];
uint128 collected = feesAccum_[token];
feesAccum_[token] = 0;
if (collected > 0) {
/**
* directly deposit token to fee protocol collector
*/
bytes32 payoutKey = keccak256(abi.encode(PROTOCOL_FEES_RECEIVER_HASH, token));
userBals_[payoutKey].surplusCollateral_ += collected;
require(userBals_[payoutKey].surplusCollateral_ <= type(uint96).max, "Value exceeds uint96 range");

uint256 amountToTransfer = uint96(userBals_[payoutKey].surplusCollateral_);

IERC20Minimal(token).approve(treasury_, amountToTransfer);
IFeeProtocolCollector(treasury_).transferTokens(token, uint96(amountToTransfer));
userBals_[payoutKey].surplusCollateral_ = 0;
}
}
}
}
5 changes: 5 additions & 0 deletions contracts/test/MockTimelock.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
pragma solidity 0.8.19;

import "../governance/SdexPolicy.sol";
import "../SdexSwapDex.sol";
import "hardhat/console.sol";

contract MockTimelock {
Expand Down Expand Up @@ -47,4 +48,8 @@ contract MockTimelock {
return SdexPolicy(policy_).setPolicy(conduit, proxyPath, policy);
}

function userCmd (address minion, uint16 proxyPath,
bytes calldata cmd) public {
SdexSwapDex(minion).userCmd(proxyPath, cmd);
}
}
4 changes: 2 additions & 2 deletions contracts/test/TestProtocolAcct.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@ contract TestProtocolAccount is ProtocolAccount {
return feesAccum_[token];
}

function disburseProtocol (address recv, address token) public {
disburseProtocolFees(recv, token);
function disburseProtocol (address[] memory tokens) public {
disburseProtocolFees(tokens);
}

function getPaidFees (address recv, address token) public view returns (uint128) {
Expand Down
37 changes: 16 additions & 21 deletions test/TestPool.govern.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,44 +148,39 @@ describe('Pool Governance', () => {
})


it("collect treasury", async() => {
it("successfully collect treasury without time delay", async() => {
await test.testRevisePool(feeRate, 128, 1) // Turn on protocol fee
await pool.connect(await test.auth).protocolCmd(test.COLD_PROXY, transferCmd(policy.address), true)
await pool.connect(await test.auth).protocolCmd(test.COLD_PROXY, transferCmd(policy2.address), true)
Copy link
Contributor

@tjcloa tjcloa Jun 26, 2024

Choose a reason for hiding this comment

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

why is policy replaced with policy2 here?

await test.testMintAmbient(10000)
await test.testSwap(true, false, 100000, MAX_PRICE)

await treasury.treasuryResolution(pool.address, test.COLD_PROXY, treasurySetCmd(policy2.address), true)
await hre.ethers.provider.send("evm_increaseTime", [3600*24*7+1]) // 7 days
await treasury2.treasuryResolution(pool.address, test.COLD_PROXY, treasurySetCmd(treasury2.address), true)

// Unauthorized attempts to collect treasury
await expect(pool.protocolCmd(test.COLD_PROXY, collectCmd(), true)).to.be.reverted
await expect(ops.opsResolution(pool.address, test.COLD_PROXY, collectCmd())).to.be.reverted
await expect(ops.treasuryResolution(pool.address, test.COLD_PROXY, collectCmd(), true)).to.be.reverted
await expect(treasury.userCmd(pool.address, test.COLD_PROXY, collectCmd())).to.be.revertedWith("Only Treasury")

// Successful treasury payout
let snap = await (await test.query).querySurplus(policy2.address, baseToken.address)
await treasury.treasuryResolution(pool.address, test.COLD_PROXY, collectCmd(), true)
expect(await (await test.query).querySurplus(policy2.address, baseToken.address)).to.gt(snap);
let snap = await (await test.query).querySurplus(treasury2.address, baseToken.address)
await treasury2.userCmd(pool.address, test.COLD_PROXY, collectCmd())
expect(await (await test.query).querySurplus(treasury2.address, baseToken.address)).to.gt(snap);
})

it("collect treasury time delay", async() => {
it("successfully collect treasury with time delay", async() => {
await test.testRevisePool(feeRate, 128, 1) // Turn on protocol fee
await pool.connect(await test.auth).protocolCmd(test.COLD_PROXY, transferCmd(policy.address), true)
await pool.connect(await test.auth).protocolCmd(test.COLD_PROXY, transferCmd(policy2.address), true)
await test.testMintAmbient(10000)
await test.testSwap(true, false, 100000, MAX_PRICE)

await treasury.treasuryResolution(pool.address, test.COLD_PROXY, treasurySetCmd(policy2.address), true)
await treasury2.treasuryResolution(pool.address, test.COLD_PROXY, treasurySetCmd(treasury2.address), true)

// Will fail because treasury can only be collected 7 days after treasury address is set
await expect(treasury.treasuryResolution(pool.address, test.COLD_PROXY, collectCmd(), true)).to.be.reverted
await hre.ethers.provider.send("evm_increaseTime", [3600*24*6]) // 6 days
await expect(treasury.treasuryResolution(pool.address, test.COLD_PROXY, collectCmd(), true)).to.be.reverted
await hre.ethers.provider.send("evm_increaseTime", [3600*24+1]) // One more day... treasury valid
// Unauthorized attempts to collect treasury
await expect(treasury.userCmd(pool.address, test.COLD_PROXY, collectCmd())).to.be.revertedWith("Only Treasury")

await hre.ethers.provider.send("evm_increaseTime", [3600*24*6]) // 6 days
// Successful treasury payout
let snap = await (await test.query).querySurplus(policy2.address, baseToken.address)
await treasury.treasuryResolution(pool.address, test.COLD_PROXY, collectCmd(), true)
expect(await (await test.query).querySurplus(policy2.address, baseToken.address)).to.gt(snap);
let snap = await (await test.query).querySurplus(treasury2.address, baseToken.address)
await treasury2.userCmd(pool.address, test.COLD_PROXY, collectCmd())
expect(await (await test.query).querySurplus(treasury2.address, baseToken.address)).to.gt(snap);
})


Expand Down