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

Jump to bottom

Fix issue with keeper compensation #451

Merged
merged 15 commits into from
Oct 4, 2024
Merged

Fix issue with keeper compensation #451

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
9aa7e03
integrated Arjun's changes
EdNoepel Aug 30, 2024
5dc8ced
gas measurement for trigger orders
EdNoepel Aug 30, 2024
d11953a
keeperConfig for withdrawals
EdNoepel Aug 30, 2024
0db3380
measure gas spent in the TX
EdNoepel Aug 31, 2024
11cb05a
compare gas spent under test to DSU fee earned by keeper
EdNoepel Sep 4, 2024
3da24a7
handle multiple commitments
EdNoepel Sep 4, 2024
d7c99ba
remove logging
EdNoepel Sep 4, 2024
78365e5
add buffered keepConfig to trigger orders
EdNoepel Sep 4, 2024
20a7e97
moved collateral account keeper configs to initializer
EdNoepel Sep 25, 2024
c651ec4
prevent double-read of order needed for keep modifier
EdNoepel Sep 5, 2024
e6a2494
update test to reconcile fees against consumed gas
EdNoepel Sep 5, 2024
e4bed21
unrelated spelling correction
EdNoepel Sep 5, 2024
30a8c29
remove unused import
EdNoepel Sep 26, 2024
6fb2320
Merge branch 'v2.3-fix-review' into ed/92-keeper-configs
EdNoepel Oct 1, 2024
2b43fc8
Merge branch 'v2.3-fix-review' into ed/92-keeper-configs
EdNoepel Oct 4, 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
7 changes: 5 additions & 2 deletions packages/perennial-account/contracts/Controller_Arbitrum.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,11 @@ import { Controller_Incentivized } from "./Controller_Incentivized.sol";
contract Controller_Arbitrum is Controller_Incentivized, Kept_Arbitrum {
/// @dev Creates instance of Controller which compensates keepers
/// @param implementation Pristine collateral account contract
constructor(address implementation, IVerifierBase nonceManager)
Controller_Incentivized(implementation, nonceManager) {}
/// @param nonceManager Verifier contract to which nonce and group cancellations are relayed
constructor(
address implementation,
IVerifierBase nonceManager
) Controller_Incentivized(implementation, nonceManager) {}

/// @dev Use the Kept_Arbitrum implementation for calculating the dynamic fee
function _calldataFee(
Expand Down
201 changes: 152 additions & 49 deletions packages/perennial-account/contracts/Controller_Incentivized.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,79 +34,117 @@ import { Withdrawal } from "./types/Withdrawal.sol";
abstract contract Controller_Incentivized is Controller, IRelayer, Kept {
/// @dev Handles relayed messages for nonce cancellation
IVerifierBase public immutable nonceManager;

/// @dev Configuration used to calculate keeper compensation
KeepConfig public keepConfig;

/// @dev Configuration used to calculate keeper compensation with buffered gas
KeepConfig public keepConfigBuffered;

/// @dev Configuration used to calculate keeper compensation to withdraw from a collateral account
KeepConfig public keepConfigWithdrawal;

/// @dev Creates instance of Controller which compensates keepers
/// @param implementation_ Pristine collateral account contract
constructor(address implementation_, IVerifierBase nonceManager_)
Controller(implementation_) {
/// @param nonceManager_ Verifier contract to which nonce and group cancellations are relayed
constructor(
address implementation_,
IVerifierBase nonceManager_
) Controller(implementation_) {
nonceManager = nonceManager_;
}

/// @notice Configures message verification and keeper compensation
/// @param marketFactory_ Contract used to validate delegated signers
/// @param verifier_ Contract used to validate collateral account message signatures
/// @param chainlinkFeed_ ETH-USD price feed used for calculating keeper compensation
/// @param keepConfig_ Configuration used to compensate keepers
/// @param keepConfig_ Configuration used for unbuffered keeper compensation
/// @param keepConfigBuffered_ Configuration used for buffered keeper compensation
/// @param keepConfigWithdrawal_ Configuration used to compensate keepers for withdrawals
function initialize(
IMarketFactory marketFactory_,
IAccountVerifier verifier_,
AggregatorV3Interface chainlinkFeed_,
KeepConfig memory keepConfig_
KeepConfig memory keepConfig_,
KeepConfig memory keepConfigBuffered_,
KeepConfig memory keepConfigWithdrawal_
) external initializer(1) {
__Factory__initialize();
__Kept__initialize(chainlinkFeed_, DSU);
marketFactory = marketFactory_;
verifier = verifier_;
keepConfig = keepConfig_;
keepConfigBuffered = keepConfigBuffered_;
keepConfigWithdrawal = keepConfigWithdrawal_;
}

/// @inheritdoc IController
function changeRebalanceConfigWithSignature(
RebalanceConfigChange calldata configChange,
bytes calldata signature
) override external {
)
external
override
keepCollateralAccount(
configChange.action.common.account,
abi.encode(configChange, signature),
configChange.action.maxFee,
0
)
{
_changeRebalanceConfigWithSignature(configChange, signature);
_compensateKeeper(configChange.action);
}

/// @inheritdoc IController
function deployAccountWithSignature(
DeployAccount calldata deployAccount_,
bytes calldata signature
) override external {
IAccount account = _deployAccountWithSignature(deployAccount_, signature);
bytes memory data = abi.encode(address(account), deployAccount_.action.maxFee);
_handleKeeperFee(keepConfig, 0, msg.data[0:0], 0, data);
)
external
override
keepCollateralAccount(
deployAccount_.action.common.account,
abi.encode(deployAccount_, signature),
deployAccount_.action.maxFee,
0
)
{
_deployAccountWithSignature(deployAccount_, signature);
}

/// @inheritdoc IController
function marketTransferWithSignature(
MarketTransfer calldata marketTransfer,
bytes calldata signature
) override external {
)
external
override
keepCollateralAccount(
marketTransfer.action.common.account,
abi.encode(marketTransfer, signature),
marketTransfer.action.maxFee,
1
)
{
IAccount account = IAccount(getAccountAddress(marketTransfer.action.common.account));
bytes memory data = abi.encode(account, marketTransfer.action.maxFee);

// if we're depositing collateral to the market, pay the keeper before transferring funds
if (marketTransfer.amount.gte(Fixed6Lib.ZERO)) {
_handleKeeperFee(keepConfig, 0, msg.data[0:0], 0, data);
_marketTransferWithSignature(account, marketTransfer, signature);
// otherwise handle the keeper fee normally, after withdrawing to the collateral account
} else {
_marketTransferWithSignature(account, marketTransfer, signature);
_handleKeeperFee(keepConfig, 0, msg.data[0:0], 0, data);
}
_marketTransferWithSignature(account, marketTransfer, signature);
Copy link
Collaborator

Choose a reason for hiding this comment

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

how are we handling deposits now that we always charge the fee afterwards?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

A "full deposit" is a corner case because fees are always paid from the account. If a user was able to transfer their entire collateral account into one or more markets, they could not pay a keeper to rebalance. When we last discussed this, we concluded an accurate gas calculation is worth the UX tradeoff.

}

/// @inheritdoc IController
function rebalanceGroup(address owner, uint256 group) override external {
function rebalanceGroup(
address owner,
uint256 group
)
external
override
keepCollateralAccount(
owner,
abi.encode(owner, group),
groupToMaxRebalanceFee[owner][group],
groupToMarkets[owner][group].length
)
{
_rebalanceGroup(owner, group);
address account = getAccountAddress(owner);
bytes memory data = abi.encode(account, groupToMaxRebalanceFee[owner][group]);
_handleKeeperFee(keepConfig, 0, msg.data[0:0], 0, data);
}

/// @inheritdoc IController
Expand All @@ -117,7 +155,13 @@ abstract contract Controller_Incentivized is Controller, IRelayer, Kept {
address account = getAccountAddress(withdrawal.action.common.account);
// levy fee prior to withdrawal
bytes memory data = abi.encode(account, withdrawal.action.maxFee);
_handleKeeperFee(keepConfig, 0, msg.data[0:0], 0, data);
_handleKeeperFee(
keepConfigWithdrawal,
0, // no way to calculate applicable gas prior to invocation
abi.encode(withdrawal, signature),
0,
data
);
_withdrawWithSignature(IAccount(account), withdrawal, signature);
}

Expand All @@ -126,13 +170,20 @@ abstract contract Controller_Incentivized is Controller, IRelayer, Kept {
RelayedNonceCancellation calldata message,
bytes calldata outerSignature,
bytes calldata innerSignature
) override external {
)
external
override
keepCollateralAccount(
message.action.common.account,
abi.encode(message, outerSignature, innerSignature),
message.action.maxFee,
0
)
{
// ensure the message was signed by the owner or a delegated signer
verifier.verifyRelayedNonceCancellation(message, outerSignature);
_ensureValidSigner(message.action.common.account, message.action.common.signer);

_compensateKeeper(message.action);

// relay the message to Verifier
nonceManager.cancelNonceWithSignature(message.nonceCancellation, innerSignature);
}
Expand All @@ -142,13 +193,20 @@ abstract contract Controller_Incentivized is Controller, IRelayer, Kept {
RelayedGroupCancellation calldata message,
bytes calldata outerSignature,
bytes calldata innerSignature
) override external {
)
external
override
keepCollateralAccount(
message.action.common.account,
abi.encode(message, outerSignature, innerSignature),
message.action.maxFee,
0
)
{
// ensure the message was signed by the owner or a delegated signer
verifier.verifyRelayedGroupCancellation(message, outerSignature);
_ensureValidSigner(message.action.common.account, message.action.common.signer);

_compensateKeeper(message.action);

// relay the message to Verifier
nonceManager.cancelGroupWithSignature(message.groupCancellation, innerSignature);
}
Expand All @@ -158,13 +216,20 @@ abstract contract Controller_Incentivized is Controller, IRelayer, Kept {
RelayedOperatorUpdate calldata message,
bytes calldata outerSignature,
bytes calldata innerSignature
) override external {
)
external
override
keepCollateralAccount(
message.action.common.account,
abi.encode(message, outerSignature, innerSignature),
message.action.maxFee,
0
)
{
// ensure the message was signed by the owner or a delegated signer
verifier.verifyRelayedOperatorUpdate(message, outerSignature);
_ensureValidSigner(message.action.common.account, message.action.common.signer);

_compensateKeeper(message.action);

// relay the message to MarketFactory
marketFactory.updateOperatorWithSignature(message.operatorUpdate, innerSignature);
}
Expand All @@ -174,13 +239,20 @@ abstract contract Controller_Incentivized is Controller, IRelayer, Kept {
RelayedSignerUpdate calldata message,
bytes calldata outerSignature,
bytes calldata innerSignature
) override external {
)
external
override
keepCollateralAccount(
message.action.common.account,
abi.encode(message, outerSignature, innerSignature),
message.action.maxFee,
0
)
{
// ensure the message was signed by the owner or a delegated signer
verifier.verifyRelayedSignerUpdate(message, outerSignature);
_ensureValidSigner(message.action.common.account, message.action.common.signer);

_compensateKeeper(message.action);

// relay the message to MarketFactory
marketFactory.updateSignerWithSignature(message.signerUpdate, innerSignature);
}
Expand All @@ -190,22 +262,24 @@ abstract contract Controller_Incentivized is Controller, IRelayer, Kept {
RelayedAccessUpdateBatch calldata message,
bytes calldata outerSignature,
bytes calldata innerSignature
) override external {
)
external
override
keepCollateralAccount(
message.action.common.account,
abi.encode(message, outerSignature, innerSignature),
message.action.maxFee,
0
)
{
// ensure the message was signed by the owner or a delegated signer
verifier.verifyRelayedAccessUpdateBatch(message, outerSignature);
_ensureValidSigner(message.action.common.account, message.action.common.signer);

_compensateKeeper(message.action);

// relay the message to MarketFactory
marketFactory.updateAccessBatchWithSignature(message.accessUpdateBatch, innerSignature);
}

function _compensateKeeper(Action calldata action) internal virtual {
bytes memory data = abi.encode(getAccountAddress(action.common.account), action.maxFee);
_handleKeeperFee(keepConfig, 0, msg.data[0:0], 0, data);
}

/// @dev Transfers funds from collateral account to controller, and limits compensation
/// to the user-defined maxFee in the Action message
/// @param amount Calculated keeper fee
Expand All @@ -224,4 +298,33 @@ abstract contract Controller_Incentivized is Controller, IRelayer, Kept {
// transfer DSU to the Controller, such that Kept can transfer to keeper
DSU.pull(account, raisedKeeperFee);
}
}

modifier keepCollateralAccount(
address account,
bytes memory applicableCalldata,
UFixed6 maxFee,
uint256 bufferMultiplier
) {
bytes memory data = abi.encode(getAccountAddress(account), maxFee);
uint256 startGas = gasleft();

_;

uint256 applicableGas = startGas - gasleft();

_handleKeeperFee(
bufferMultiplier > 0
? KeepConfig(
keepConfigBuffered.multiplierBase,
keepConfigBuffered.bufferBase * (bufferMultiplier),
keepConfigBuffered.multiplierCalldata,
keepConfigBuffered.bufferCalldata * (bufferMultiplier)
)
: keepConfig,
applicableGas,
applicableCalldata,
0,
data
);
}
}
1 change: 0 additions & 1 deletion packages/perennial-account/test/helpers/arbitrumHelpers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import {
IOracleProvider,
GasOracle__factory,
} from '../../types/generated'
import { IKept } from '../../types/generated/contracts/Controller_Arbitrum'
import { impersonate } from '../../../common/testutil'
import { IVerifier } from '@equilibria/perennial-v2/types/generated'

Expand Down
Loading
Loading