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.

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

billing config override #12536

Merged
merged 2 commits into from
Mar 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 5 additions & 0 deletions .changeset/modern-candles-begin.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"chainlink": patch
---

billing overrides
5 changes: 5 additions & 0 deletions contracts/.changeset/happy-books-taste.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@chainlink/contracts": patch
---

billing overrides

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,7 @@ contract SetConfig is SetUp {
fallbackNativePrice: 400_000_000_000, // $4,000
transcoder: 0xB1e66855FD67f6e85F0f0fA38cd6fBABdf00923c,
registrars: new address[](0),
upkeepPrivilegeManager: 0xD9c855F08A7e460691F41bBDDe6eC310bc0593D8,
upkeepPrivilegeManager: PRIVILEGE_MANAGER,
chainModule: module,
reorgProtectionEnabled: true,
financeAdmin: FINANCE_ADMIN
Expand Down Expand Up @@ -900,3 +900,52 @@ contract GetMinBalanceForUpkeep is SetUp {
assertEq(minBalanceAfter, minBalanceBefore + (uint256(usdTokenConfig.flatFeeMilliCents) * 1e13));
}
}

contract BillingOverrides is SetUp {
event BillingConfigOverridden(uint256 indexed id);
event BillingConfigOverrideRemoved(uint256 indexed id);

function test_RevertsWhen_NotPrivilegeManager() public {
AutomationRegistryBase2_3.BillingOverrides memory billingOverrides = AutomationRegistryBase2_3.BillingOverrides({
gasFeePPB: 5_000,
flatFeeMilliCents: 20_000
});

vm.expectRevert(Registry.OnlyCallableByUpkeepPrivilegeManager.selector);
registry.setBillingOverrides(linkUpkeepID, billingOverrides);
}

function test_RevertsWhen_UpkeepCancelled() public {
AutomationRegistryBase2_3.BillingOverrides memory billingOverrides = AutomationRegistryBase2_3.BillingOverrides({
gasFeePPB: 5_000,
flatFeeMilliCents: 20_000
});

registry.cancelUpkeep(linkUpkeepID);

vm.startPrank(PRIVILEGE_MANAGER);
vm.expectRevert(Registry.UpkeepCancelled.selector);
registry.setBillingOverrides(linkUpkeepID, billingOverrides);
}

function test_Happy_SetBillingOverrides() public {
AutomationRegistryBase2_3.BillingOverrides memory billingOverrides = AutomationRegistryBase2_3.BillingOverrides({
gasFeePPB: 5_000,
flatFeeMilliCents: 20_000
});

vm.startPrank(PRIVILEGE_MANAGER);

vm.expectEmit();
emit BillingConfigOverridden(linkUpkeepID);
registry.setBillingOverrides(linkUpkeepID, billingOverrides);
}

function test_Happy_RemoveBillingOverrides() public {
vm.startPrank(PRIVILEGE_MANAGER);

vm.expectEmit();
emit BillingConfigOverrideRemoved(linkUpkeepID);
registry.removeBillingOverrides(linkUpkeepID);
}
}
3 changes: 2 additions & 1 deletion contracts/src/v0.8/automation/dev/test/BaseTest.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ contract BaseTest is Test {
address internal constant FINANCE_ADMIN = address(uint160(uint256(keccak256("FINANCE_ADMIN"))));
address internal constant STRANGER = address(uint160(uint256(keccak256("STRANGER"))));
address internal constant BROKE_USER = address(uint160(uint256(keccak256("BROKE_USER")))); // do not mint to this address
address internal constant PRIVILEGE_MANAGER = address(uint160(uint256(keccak256("PRIVILEGE_MANAGER"))));

// nodes
uint256 internal constant SIGNING_KEY0 = 0x7b2e97fe057e6de99d6872a2ef2abf52c9b4469bc848c2465ac3fcd8d336e81d;
Expand Down Expand Up @@ -222,7 +223,7 @@ contract BaseTest is Test {
fallbackNativePrice: 400_000_000_000, // $4,000
transcoder: 0xB1e66855FD67f6e85F0f0fA38cd6fBABdf00923c,
registrars: registrars,
upkeepPrivilegeManager: 0xD9c855F08A7e460691F41bBDDe6eC310bc0593D8,
upkeepPrivilegeManager: PRIVILEGE_MANAGER,
chainModule: address(new ChainModuleBase()),
reorgProtectionEnabled: true,
financeAdmin: FINANCE_ADMIN
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,8 @@ contract AutomationRegistry2_3 is AutomationRegistryBase2_3, OCR2Abstract, Chain
billingToken: billingTokenParams,
isTransaction: true
}),
report.upkeepIds[i]
report.upkeepIds[i],
upkeepTransmitInfo[i].upkeep
);
transmitVars.totalPremium += receipt.premiumJuels;
transmitVars.totalReimbursement += receipt.gasReimbursementJuels;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ abstract contract AutomationRegistryBase2_3 is ConfirmedOwner {
mapping(address => bytes) internal s_adminPrivilegeConfig; // general config set by an administrative role for an admin
// billing
mapping(IERC20 billingToken => BillingConfig billingConfig) internal s_billingConfigs; // billing configurations for different tokens
mapping(uint256 upkeepID => BillingOverrides billingOverrides) internal s_billingOverrides; // billing overrides for specific upkeeps
IERC20[] internal s_billingTokens; // list of billing tokens
PayoutMode internal s_payoutMode;

Expand Down Expand Up @@ -242,6 +243,7 @@ abstract contract AutomationRegistryBase2_3 is ConfirmedOwner {
/**
* @notice relevant state of an upkeep which is used in transmit function
* @member paused if this upkeep has been paused
* @member overridesEnabled if this upkeep has overrides enabled
* @member performGas the gas limit of upkeep execution
* @member maxValidBlocknumber until which block this upkeep is valid
* @member forwarder the forwarder contract to use for this upkeep
Expand All @@ -251,10 +253,11 @@ abstract contract AutomationRegistryBase2_3 is ConfirmedOwner {
*/
struct Upkeep {
bool paused;
bool overridesEnabled;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note: this newly added field will not exist in IAutomationRegistryMaster2_3.sol -> UpkeepInfoLegacy. But this shouldnt be a backward compatible issue.

uint32 performGas;
uint32 maxValidBlocknumber;
IAutomationForwarder forwarder;
// 3 bytes left in 1st EVM word - read in transmit path
// 2 bytes left in 1st EVM word - read in transmit path
uint128 amountSpent;
uint96 balance;
uint32 lastPerformedBlockNumber;
Expand Down Expand Up @@ -382,7 +385,15 @@ abstract contract AutomationRegistryBase2_3 is ConfirmedOwner {
}

/**
* @notice pricing params for a biling token
* @notice override-able billing params of a billing token
*/
struct BillingOverrides {
uint32 gasFeePPB;
uint24 flatFeeMilliCents;
}

/**
* @notice pricing params for a billing token
* @dev this is a memory-only struct, so struct packing is less important
*/
struct BillingTokenPaymentParams {
Expand Down Expand Up @@ -428,6 +439,8 @@ abstract contract AutomationRegistryBase2_3 is ConfirmedOwner {
}

event AdminPrivilegeConfigSet(address indexed admin, bytes privilegeConfig);
event BillingConfigOverridden(uint256 indexed id);
Copy link
Contributor

Choose a reason for hiding this comment

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

can we add the BillingConfigOverride to this event?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you mean renaming this event from BillingConfigOverridden to BillingConfigOverride ?

Copy link
Contributor

Choose a reason for hiding this comment

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

oh sorry I meant like event BillingConfigOverridden(uint256 indexed id, BillingConfigOverride override);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Got it!

event BillingConfigOverrideRemoved(uint256 indexed id);
event CancelledUpkeepReport(uint256 indexed id, bytes trigger);
event ChainSpecificModuleUpdated(address newModule);
event DedupKeyAdded(bytes32 indexed dedupKey);
Expand Down Expand Up @@ -674,6 +687,7 @@ abstract contract AutomationRegistryBase2_3 is ConfirmedOwner {
* maximum gas overhead, L1 fee
*/
function _getMaxPayment(
uint256 upkeepId,
HotVars memory hotVars,
Trigger triggerType,
uint32 performGas,
Expand Down Expand Up @@ -704,6 +718,14 @@ abstract contract AutomationRegistryBase2_3 is ConfirmedOwner {
maxL1Fee = hotVars.gasCeilingMultiplier * hotVars.chainModule.getMaxL1Fee(maxCalldataSize);
}

BillingTokenPaymentParams memory paymentParams = _getBillingTokenPaymentParams(hotVars, billingToken);
if (s_upkeep[upkeepId].overridesEnabled) {
BillingOverrides memory billingOverrides = s_billingOverrides[upkeepId];
// use the overridden configs
paymentParams.gasFeePPB = billingOverrides.gasFeePPB;
paymentParams.flatFeeMilliCents = billingOverrides.flatFeeMilliCents;
}

PaymentReceipt memory receipt = _calculatePaymentAmount(
hotVars,
PaymentParams({
Expand All @@ -713,7 +735,7 @@ abstract contract AutomationRegistryBase2_3 is ConfirmedOwner {
fastGasWei: fastGasWei,
linkUSD: linkUSD,
nativeUSD: nativeUSD,
billingToken: _getBillingTokenPaymentParams(hotVars, billingToken),
billingToken: paymentParams,
isTransaction: false
})
);
Expand Down Expand Up @@ -941,11 +963,19 @@ abstract contract AutomationRegistryBase2_3 is ConfirmedOwner {
function _handlePayment(
HotVars memory hotVars,
PaymentParams memory paymentParams,
uint256 upkeepId
uint256 upkeepId,
Upkeep memory upkeep
) internal returns (PaymentReceipt memory) {
if (upkeep.overridesEnabled) {
BillingOverrides memory billingOverrides = s_billingOverrides[upkeepId];
// use the overridden configs
paymentParams.billingToken.gasFeePPB = billingOverrides.gasFeePPB;
paymentParams.billingToken.flatFeeMilliCents = billingOverrides.flatFeeMilliCents;
}

PaymentReceipt memory receipt = _calculatePaymentAmount(hotVars, paymentParams);

uint96 balance = s_upkeep[upkeepId].balance;
uint96 balance = upkeep.balance;
uint96 payment = receipt.gasCharge + receipt.premium;

// this shouldn't happen, but in rare edge cases, we charge the full balance in case the user
Expand Down Expand Up @@ -1007,6 +1037,15 @@ abstract contract AutomationRegistryBase2_3 is ConfirmedOwner {
}
}

/**
* @notice only allows privilege manager to call the function
*/
function _onlyPrivilegeManagerAllowed() internal view {
if (msg.sender != s_storage.upkeepPrivilegeManager) {
revert OnlyCallableByUpkeepPrivilegeManager();
}
}

/**
* @notice sets billing configuration for a token
* @param billingTokens the addresses of tokens
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ contract AutomationRegistryLogicA2_3 is AutomationRegistryBase2_3, Chainable {
if (upkeep.paused) return (false, bytes(""), UpkeepFailureReason.UPKEEP_PAUSED, 0, upkeep.performGas, 0, 0);
(fastGasWei, linkUSD, nativeUSD) = _getFeedData(hotVars);
maxPayment = _getMaxPayment(
id,
hotVars,
triggerType,
upkeep.performGas,
Expand Down Expand Up @@ -243,6 +244,7 @@ contract AutomationRegistryLogicA2_3 is AutomationRegistryBase2_3, Chainable {
_createUpkeep(
id,
Upkeep({
overridesEnabled: false,
performGas: gasLimit,
balance: 0,
maxValidBlocknumber: UINT32_MAX,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,32 @@ contract AutomationRegistryLogicB2_3 is AutomationRegistryBase2_3, Chainable {
// | UPKEEP MANAGEMENT |
// ================================================================

/**
* @notice overrides the billing config for an upkeep
* @param id the upkeepID
* @param billingOverrides the override-able billing config
*/
function setBillingOverrides(uint256 id, BillingOverrides calldata billingOverrides) external {
Copy link
Contributor

Choose a reason for hiding this comment

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

great use of calldata 👍

_onlyPrivilegeManagerAllowed();
if (s_upkeep[id].maxValidBlocknumber != UINT32_MAX) revert UpkeepCancelled();

s_upkeep[id].overridesEnabled = true;
s_billingOverrides[id] = billingOverrides;
emit BillingConfigOverridden(id);
}

/**
* @notice remove the overridden billing config for an upkeep
* @param id the upkeepID
*/
function removeBillingOverrides(uint256 id) external {
_onlyPrivilegeManagerAllowed();

s_upkeep[id].overridesEnabled = false;
delete s_billingOverrides[id];
emit BillingConfigOverrideRemoved(id);
}

/**
* @notice adds fund to an upkeep
* @param id the upkeepID
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,9 +91,7 @@ contract AutomationRegistryLogicC2_3 is AutomationRegistryBase2_3 {
* @notice sets the privilege config for an upkeep
*/
function setUpkeepPrivilegeConfig(uint256 upkeepId, bytes calldata newPrivilegeConfig) external {
if (msg.sender != s_storage.upkeepPrivilegeManager) {
revert OnlyCallableByUpkeepPrivilegeManager();
}
_onlyPrivilegeManagerAllowed();
Copy link
Contributor

Choose a reason for hiding this comment

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

nice, much cleaner 👍

s_upkeepPrivilegeConfig[upkeepId] = newPrivilegeConfig;
emit UpkeepPrivilegeConfigSet(upkeepId, newPrivilegeConfig);
}
Expand Down Expand Up @@ -147,9 +145,7 @@ contract AutomationRegistryLogicC2_3 is AutomationRegistryBase2_3 {
* @param newPrivilegeConfig the privileges that this admin has
*/
function setAdminPrivilegeConfig(address admin, bytes calldata newPrivilegeConfig) external {
if (msg.sender != s_storage.upkeepPrivilegeManager) {
revert OnlyCallableByUpkeepPrivilegeManager();
}
_onlyPrivilegeManagerAllowed();
s_adminPrivilegeConfig[admin] = newPrivilegeConfig;
emit AdminPrivilegeConfigSet(admin, newPrivilegeConfig);
}
Expand Down Expand Up @@ -495,21 +491,22 @@ contract AutomationRegistryLogicC2_3 is AutomationRegistryBase2_3 {
*/
function getMinBalanceForUpkeep(uint256 id) public view returns (uint96 minBalance) {
Upkeep memory upkeep = s_upkeep[id];
return getMaxPaymentForGas(_getTriggerType(id), upkeep.performGas, upkeep.billingToken);
return getMaxPaymentForGas(id, _getTriggerType(id), upkeep.performGas, upkeep.billingToken);
}

/**
* @notice calculates the maximum payment for a given gas limit
* @param gasLimit the gas to calculate payment for
*/
function getMaxPaymentForGas(
uint256 id,
Trigger triggerType,
uint32 gasLimit,
IERC20 billingToken
) public view returns (uint96 maxPayment) {
HotVars memory hotVars = s_hotVars;
(uint256 fastGasWei, uint256 linkUSD, uint256 nativeUSD) = _getFeedData(hotVars);
return _getMaxPayment(hotVars, triggerType, gasLimit, fastGasWei, linkUSD, nativeUSD, billingToken);
return _getMaxPayment(id, hotVars, triggerType, gasLimit, fastGasWei, linkUSD, nativeUSD, billingToken);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -672,6 +672,7 @@
)

const conditionalPrice = await registry.getMaxPaymentForGas(
upkeepId,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will use another PR to haul all those getMaxPaymentForGas ts tests to foundry, so we can also further test the overrides logic. I hope that is okay.

Copy link
Contributor

Choose a reason for hiding this comment

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

totally fine!

Trigger.CONDITION,
test.gas,
linkToken.address,
Expand All @@ -688,6 +689,7 @@
)

const logPrice = await registry.getMaxPaymentForGas(
upkeepId,
Trigger.LOG,
test.gas,
linkToken.address,
Expand Down Expand Up @@ -1815,6 +1817,7 @@

itMaybe('can self fund', async () => {
const maxPayment = await registry.getMaxPaymentForGas(
upkeepId,
Trigger.CONDITION,
performGas,
linkToken.address,
Expand Down Expand Up @@ -3596,6 +3599,7 @@
expectedFallbackMaxPayment.toString(),
(
await registry.getMaxPaymentForGas(
upkeepId,
Trigger.CONDITION,
performGas,
linkToken.address,
Expand All @@ -3615,6 +3619,7 @@
expectedFallbackMaxPayment.toString(),
(
await registry.getMaxPaymentForGas(
upkeepId,
Trigger.CONDITION,
performGas,
linkToken.address,
Expand All @@ -3634,6 +3639,7 @@
expectedFallbackMaxPayment.toString(),
(
await registry.getMaxPaymentForGas(
upkeepId,
Trigger.CONDITION,
performGas,
linkToken.address,
Expand Down Expand Up @@ -3681,6 +3687,7 @@
expectedFallbackMaxPayment.toString(),
(
await registry.getMaxPaymentForGas(
upkeepId,
Trigger.CONDITION,
performGas,
linkToken.address,
Expand All @@ -3700,6 +3707,7 @@
expectedFallbackMaxPayment.toString(),
(
await registry.getMaxPaymentForGas(
upkeepId,
Trigger.CONDITION,
performGas,
linkToken.address,
Expand All @@ -3719,6 +3727,7 @@
expectedFallbackMaxPayment.toString(),
(
await registry.getMaxPaymentForGas(
upkeepId,
Trigger.CONDITION,
performGas,
linkToken.address,
Expand All @@ -3736,8 +3745,8 @@
})

describeMaybe('#setConfig - onchain', async () => {
const payment = BigNumber.from(1)

Check warning on line 3748 in contracts/test/v0.8/automation/AutomationRegistry2_3.test.ts

View workflow job for this annotation

GitHub Actions / Solidity Lint

'payment' is assigned a value but never used. Allowed unused vars must match /^_/u
const flatFee = BigNumber.from(2)

Check warning on line 3749 in contracts/test/v0.8/automation/AutomationRegistry2_3.test.ts

View workflow job for this annotation

GitHub Actions / Solidity Lint

'flatFee' is assigned a value but never used. Allowed unused vars must match /^_/u
const maxGas = BigNumber.from(6)
const staleness = BigNumber.from(4)
const ceiling = BigNumber.from(5)
Expand Down

Large diffs are not rendered by default.

Loading
Loading