-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
support decimals #12812
support decimals #12812
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
--- | ||
"chainlink": patch | ||
--- | ||
|
||
support decimals #added |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
--- | ||
"chainlink": patch | ||
--- | ||
|
||
support decimals #added |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
--- | ||
"@chainlink/contracts": patch | ||
--- | ||
|
||
support decimals #added |
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -375,6 +375,7 @@ abstract contract AutomationRegistryBase2_3 is ConfirmedOwner { | |
uint32 gasFeePPB; | ||
uint24 flatFeeMilliCents; // min fee is $0.00001, max fee is $167 | ||
AggregatorV3Interface priceFeed; | ||
uint8 decimals; | ||
// 1st word, read in calculating BillingTokenPaymentParams | ||
uint256 fallbackPrice; | ||
// 2nd word only read if stale | ||
|
@@ -395,6 +396,7 @@ abstract contract AutomationRegistryBase2_3 is ConfirmedOwner { | |
* @dev this is a memory-only struct, so struct packing is less important | ||
*/ | ||
struct BillingTokenPaymentParams { | ||
uint8 decimals; | ||
uint32 gasFeePPB; | ||
uint24 flatFeeMilliCents; | ||
uint256 priceUSD; | ||
|
@@ -426,8 +428,8 @@ abstract contract AutomationRegistryBase2_3 is ConfirmedOwner { | |
|
||
/** | ||
* @notice struct containing receipt information about a payment or cost estimation | ||
* @member gasCharge the amount to charge a user for gas spent | ||
* @member premium the premium charged to the user, shared between all nodes | ||
* @member gasCharge the amount to charge a user for gas spent using the billing token's native decimals | ||
* @member premium the premium charged to the user, shared between all nodes, using the billing token's native decimals | ||
* @member gasReimbursementJuels the amount to reimburse a node for gas spent | ||
* @member premiumJuels the premium paid to NOPs, shared between all nodes | ||
*/ | ||
|
@@ -633,6 +635,7 @@ abstract contract AutomationRegistryBase2_3 is ConfirmedOwner { | |
BillingConfig storage config = s_billingConfigs[billingToken]; | ||
paymentParams.flatFeeMilliCents = config.flatFeeMilliCents; | ||
paymentParams.gasFeePPB = config.gasFeePPB; | ||
paymentParams.decimals = config.decimals; | ||
(, int256 feedValue, , uint256 timestamp, ) = config.priceFeed.latestRoundData(); | ||
if ( | ||
feedValue <= 0 || | ||
|
@@ -660,6 +663,7 @@ abstract contract AutomationRegistryBase2_3 is ConfirmedOwner { | |
HotVars memory hotVars, | ||
PaymentParams memory paymentParams | ||
) internal view returns (PaymentReceipt memory receipt) { | ||
uint8 decimals = paymentParams.billingTokenParams.decimals; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it's probably cheaper to cast this as a |
||
uint256 gasWei = paymentParams.fastGasWei * hotVars.gasCeilingMultiplier; | ||
// in case it's actual execution use actual gas price, capped by fastGasWei * gasCeilingMultiplier | ||
if (paymentParams.isTransaction && tx.gasprice < gasWei) { | ||
|
@@ -669,13 +673,29 @@ abstract contract AutomationRegistryBase2_3 is ConfirmedOwner { | |
uint256 gasPaymentHexaicosaUSD = (gasWei * | ||
(paymentParams.gasLimit + paymentParams.gasOverhead) + | ||
paymentParams.l1CostWei) * paymentParams.nativeUSD; // gasPaymentHexaicosaUSD has an extra 8 zeros because of decimals on nativeUSD feed | ||
receipt.gasCharge = SafeCast.toUint96(gasPaymentHexaicosaUSD / paymentParams.billingTokenParams.priceUSD); // has units of attoBillingToken, or "wei" | ||
|
||
uint96 gasCharge18Decimals = SafeCast.toUint96(gasPaymentHexaicosaUSD / paymentParams.billingTokenParams.priceUSD); // has units of attoBillingToken, or "wei" | ||
receipt.gasCharge = gasCharge18Decimals; | ||
if (decimals < 18) { | ||
receipt.gasCharge = SafeCast.toUint96(gasCharge18Decimals / (10 ** (18 - decimals))); | ||
} else if (decimals > 18) { | ||
receipt.gasCharge = SafeCast.toUint96(gasCharge18Decimals * (10 ** (decimals - 18))); | ||
} | ||
|
||
receipt.gasReimbursementJuels = SafeCast.toUint96(gasPaymentHexaicosaUSD / paymentParams.linkUSD); | ||
uint256 flatFeeHexaicosaUSD = uint256(paymentParams.billingTokenParams.flatFeeMilliCents) * 1e21; // 1e13 for milliCents to attoUSD and 1e8 for attoUSD to hexaicosaUSD | ||
uint256 premiumHexaicosaUSD = ((((gasWei * paymentParams.gasLimit) + paymentParams.l1CostWei) * | ||
paymentParams.billingTokenParams.gasFeePPB * | ||
paymentParams.nativeUSD) / 1e9) + flatFeeHexaicosaUSD; | ||
receipt.premium = SafeCast.toUint96(premiumHexaicosaUSD / paymentParams.billingTokenParams.priceUSD); | ||
|
||
uint96 premium18Decimals = SafeCast.toUint96(premiumHexaicosaUSD / paymentParams.billingTokenParams.priceUSD); // has units of attoBillingToken, or "wei" | ||
receipt.premium = premium18Decimals; | ||
if (decimals < 18) { | ||
receipt.premium = SafeCast.toUint96(premium18Decimals / (10 ** (18 - decimals))); | ||
} else if (decimals > 18) { | ||
receipt.premium = SafeCast.toUint96(premium18Decimals * (10 ** (decimals - 18))); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. because we're doing multiplication (line 696) after division (line 691) I think we're potentially losing some precision. Could we instead do something like this... uint256 numeratorScalingFactor = decimals > 18 ? 10 ** (decimals - 18) : 1;
uint256 denominatorScalingFactor = decimals < 18 ? 10 ** (18 - decimals) : 1;
receipt.premium = SafeCast.toUint96(
(premiumHexaicosaUSD * numeratorScalingFactor) / (paymentParams.billingTokenParams.priceUSD * denominatorScalingFactor )
); wdyt? |
||
} | ||
|
||
receipt.premiumJuels = SafeCast.toUint96(premiumHexaicosaUSD / paymentParams.linkUSD); | ||
|
||
return receipt; | ||
|
@@ -975,7 +995,9 @@ abstract contract AutomationRegistryBase2_3 is ConfirmedOwner { | |
|
||
PaymentReceipt memory receipt = _calculatePaymentAmount(hotVars, paymentParams); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. in the
|
||
|
||
// balance is in the token's native decimals | ||
uint96 balance = upkeep.balance; | ||
// payment is in the token's native decimals | ||
uint96 payment = receipt.gasCharge + receipt.premium; | ||
|
||
// this shouldn't happen, but in rare edge cases, we charge the full balance in case the user | ||
|
@@ -1064,6 +1086,11 @@ abstract contract AutomationRegistryBase2_3 is ConfirmedOwner { | |
IERC20 token = billingTokens[i]; | ||
BillingConfig memory config = billingConfigs[i]; | ||
|
||
// most ERC20 tokens are 18 decimals, we support tokens with up to 24 decimals | ||
if (config.decimals > 24) { | ||
revert InvalidToken(); | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why only up to 24? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I checked several mainnet tokens and found that all of them have a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here can we also make a requirement that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
24 is just a number, we can change.
I am pretty confident that this decimals() can be done!
Compiler will not be happy with this. our billingToken is of type IERC20 which doesnt have decimals(). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry, typo in the above message, the billingConfig.aggregator decimals should be 8 not 18 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Can we change from IERC20 to something that supports the decimals() function? How about this guy? We can alias the import so it's a little cleaner... import {IERC20Metadata as IERC20} from ... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I will explore this IERC20Metadata. There is definitely way to get the decimals. If OZ's interfaces dont provide that, we can make a low level call from the token address and fetch the decimals if it exists.
|
||
|
||
// if LINK is a billing option, payout mode must be ON_CHAIN | ||
if (address(token) == address(i_link) && mode == PayoutMode.OFF_CHAIN) { | ||
revert InvalidToken(); | ||
|
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably only need one changeset file 😆