support billing config #12310
support billing config #12310
Conversation
|
I see that you haven't updated any README files. Would it make sense to do so? |
|
Go solidity wrappers are out-of-date, regenerate them via the |
shileiwill
force-pushed
the
AUTO-9111-new2
branch
from
e893c42 to
932d372
Compare
| uint96 oldLength = uint96(s_transmittersList.length); | ||
| for (uint256 i = 0; i < oldLength; i++) { | ||
| _updateTransmitterBalanceFromPool(s_transmittersList[i], totalPremium, oldLength); | ||
| for (uint256 i = 0; i < s_transmittersList.length; i++) { |
There was a problem hiding this comment.
no need to access .length so many times?
There was a problem hiding this comment.
Good Q, removed the oldLength variable to avoid stack too deep. confirmed with Ryan, a bit more gas is not a concern in this non-transmit path.
shileiwill
force-pushed
the
AUTO-9111-new2
branch
from
7bfb904 to
4a6e825
Compare
shileiwill
force-pushed
the
AUTO-9111-new2
branch
from
4a6e825 to
29aec15
Compare
shileiwill
force-pushed
the
AUTO-9111-new2
branch
from
29aec15 to
a18f112
Compare
|
Go solidity wrappers are out-of-date, regenerate them via the |
1 similar comment
|
Go solidity wrappers are out-of-date, regenerate them via the |
shileiwill
force-pushed
the
AUTO-9111-new2
branch
from
8103f53 to
6854a6c
Compare
shileiwill
force-pushed
the
AUTO-9111-new2
branch
from
6854a6c to
620e9a2
Compare
| @@ -155,6 +158,7 @@ abstract contract AutomationRegistryBase2_3 is ConfirmedOwner { | |||
| error UpkeepNotCanceled(); | |||
| error UpkeepNotNeeded(); | |||
| error ValueNotChanged(); | |||
| error ZeroAddressNotAllowed(); | |||
There was a problem hiding this comment.
👍 for generic / reusable errors
| @@ -677,7 +693,7 @@ abstract contract AutomationRegistryBase2_3 is ConfirmedOwner { | |||
| function _updateTransmitterBalanceFromPool( | |||
| address transmitterAddress, | |||
| uint96 totalPremium, | |||
| uint96 payeeCount | |||
| uint96 payeeCount // TODO: why payeeCount is of type uint96? | |||
There was a problem hiding this comment.
meh I don't love it but let's not fix what's not broken eh? I think remove the comment and we can save this for a future version.
There was a problem hiding this comment.
created https://smartcontract-it.atlassian.net/browse/AUTO-9235 and added to Misc Contract 2.2+.
| if (s_billingConfigs[token].priceFeed == address(0)) { | ||
| s_billingTokens.push(token); | ||
| } |
There was a problem hiding this comment.
can we have an else { revert DuplicateEntry() } here?
There was a problem hiding this comment.
or maybe simpler...
if (s_billingConfigs[token].priceFeed != address(0)) {
revert DuplicateEntry();
}
s_billingTokens.push(token);
There was a problem hiding this comment.
Got it, we will not swallow the duplicate entries, we will revert!
| * @notice the billing config of a token | ||
| */ | ||
| struct BillingConfig { | ||
| bool active; |
There was a problem hiding this comment.
I think we can get rid of this field! I don't forsee it being necessary.
| mapping(address billingToken => BillingConfig billingConfig) internal s_billingConfigs; // billing configurations for different tokens | ||
| address[] internal s_billingTokens; // list of billing tokens |
There was a problem hiding this comment.
one more comment - can we make these IERC20s instead of addresss? you can get that from vendor/openzeppelin-solidity/v4.8.3/contracts/token/ERC20/IERC20.sol
There was a problem hiding this comment.
Sure, that makes more sense. I guess we should use IERC20 for both the mapping and the list
There was a problem hiding this comment.
yeah pretty much everywhere - including in the functions like getBillingConfig() and setConfig(), etc
There was a problem hiding this comment.
I updated the underlying data structures and associated APIs. I didnt touch the setConfigTypeSafe signature tho, let me know if you think that need to use IERC20 as well.
There was a problem hiding this comment.
yeah I think make that IERC20 also!
shileiwill
force-pushed
the
AUTO-9111-new2
branch
from
620e9a2 to
412dff6
Compare
|
Go solidity wrappers are out-of-date, regenerate them via the |
| @@ -483,6 +497,8 @@ abstract contract AutomationRegistryBase2_3 is ConfirmedOwner { | |||
| event UpkeepTriggerConfigSet(uint256 indexed id, bytes triggerConfig); | |||
| event UpkeepUnpaused(uint256 indexed id); | |||
| event Unpaused(address account); | |||
| // Event to emit when a billing configuration is set | |||
| event BillingConfigSet(IERC20 indexed token, BillingConfig config); | |||
| * @param billingTokens the addresses of tokens | ||
| * @param billingConfigs the configs for tokens | ||
| */ | ||
| function _setBillingConfig(address[] memory billingTokens, BillingConfig[] memory billingConfigs) internal { |
| delete s_billingTokens; | ||
|
|
||
| for (uint256 i = 0; i < billingTokens.length; i++) { | ||
| IERC20 token = IERC20(billingTokens[i]); |
There was a problem hiding this comment.
then we can delete this line :)
| @@ -266,23 +273,30 @@ contract AutomationRegistry2_3 is AutomationRegistryBase2_3, OCR2Abstract, Chain | |||
| uint8 f, | |||
| OnchainConfig memory onchainConfig, | |||
| uint64 offchainConfigVersion, | |||
| bytes memory offchainConfig | |||
| bytes memory offchainConfig, | |||
| address[] memory billingTokens, | |||
shileiwill
force-pushed
the
AUTO-9111-new2
branch
from
b467675 to
ab026cb
Compare
|
* support billing config * use IERC20
* support billing config * use IERC20
AUTO-9111
This is a copy of this PR. Some great comments from Ryan are from the old PR, but unfortunately I messed up that PR when rebasing.
All comments should have been addressed except this one pending confirmation: #12295 (comment)