Skip to content

Commit

Permalink
some contract v2.3 tuning (#12790)
Browse files Browse the repository at this point in the history 
* some contract v2.3 tuning

* address comments
  • Loading branch information
@shileiwill
shileiwill authored Apr 12, 2024
1 parent a3c8a86 commit 521a035
Show file tree
Hide file tree
Showing 16 changed files with 209 additions and 120 deletions.
5 changes: 5 additions & 0 deletions .changeset/odd-horses-fix.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"chainlink": patch
---

contracts work
5 changes: 5 additions & 0 deletions contracts/.changeset/tasty-kangaroos-approve.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@chainlink/contracts": patch
---

contracts work
6 changes: 3 additions & 3 deletions contracts/src/v0.8/automation/dev/interfaces/v2_3/IAutomationRegistryMaster2_3.sol
View file

Large diffs are not rendered by default.

30 changes: 30 additions & 0 deletions contracts/src/v0.8/automation/dev/test/AutomationRegistrar2_3.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -211,4 +211,34 @@ contract RegisterUpkeep is SetUp {
assertEq(weth.balanceOf(address(registrar)), amount);
assertEq(registry.getNumUpkeeps(), 0);
}

function testLink_autoApproveOff_revertOnDuplicateEntry() external {
vm.startPrank(UPKEEP_ADMIN);

uint96 amount = uint96(registrar.getMinimumRegistrationAmount(IERC20(address(linkToken))));
linkToken.approve(address(registrar), amount * 2);

AutomationRegistrar2_3.RegistrationParams memory params = AutomationRegistrar2_3.RegistrationParams({
upkeepContract: address(TARGET1),
amount: amount,
adminAddress: UPKEEP_ADMIN,
gasLimit: 10_000,
triggerType: 0,
billingToken: IERC20(address(linkToken)),
name: "foobar",
encryptedEmail: "",
checkData: bytes("check data"),
triggerConfig: "",
offchainConfig: ""
});

registrar.registerUpkeep(params);

assertEq(linkToken.balanceOf(address(registrar)), amount);
assertEq(registry.getNumUpkeeps(), 0);

// attempt to register the same upkeep again
vm.expectRevert(AutomationRegistrar2_3.DuplicateEntry.selector);
registrar.registerUpkeep(params);
}
}
100 changes: 68 additions & 32 deletions contracts/src/v0.8/automation/dev/test/AutomationRegistry2_3.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ contract SetUp is BaseTest {
bytes internal constant offchainConfigBytes = abi.encode(1234, ZERO_ADDRESS);

uint256 linkUpkeepID;
uint256 linkUpkeepID2;
uint256 usdUpkeepID;
uint256 nativeUpkeepID;

Expand Down Expand Up @@ -57,6 +58,17 @@ contract SetUp is BaseTest {
""
);

linkUpkeepID2 = registry.registerUpkeep(
address(TARGET1),
config.maxPerformGas,
UPKEEP_ADMIN,
uint8(Trigger.CONDITION),
address(linkToken),
"",
"",
""
);

usdUpkeepID = registry.registerUpkeep(
address(TARGET1),
config.maxPerformGas,
Expand All @@ -81,6 +93,7 @@ contract SetUp is BaseTest {

vm.startPrank(OWNER);
registry.addFunds(linkUpkeepID, registry.getMinBalanceForUpkeep(linkUpkeepID));
registry.addFunds(linkUpkeepID2, registry.getMinBalanceForUpkeep(linkUpkeepID2));
registry.addFunds(usdUpkeepID, registry.getMinBalanceForUpkeep(usdUpkeepID));
registry.addFunds(nativeUpkeepID, registry.getMinBalanceForUpkeep(nativeUpkeepID));
vm.stopPrank();
Expand Down Expand Up @@ -160,21 +173,21 @@ contract AddFunds is SetUp {
function test_movesFundFromCorrectToken() public {
vm.startPrank(UPKEEP_ADMIN);

uint256 startBalanceLINK = linkToken.balanceOf(address(registry));
uint256 startBalanceUSDToken = usdToken.balanceOf(address(registry));
uint256 startLINKRegistryBalance = linkToken.balanceOf(address(registry));
uint256 startUSDRegistryBalance = usdToken.balanceOf(address(registry));
uint256 startLinkUpkeepBalance = registry.getBalance(linkUpkeepID);
uint256 startUSDUpkeepBalance = registry.getBalance(usdUpkeepID);

registry.addFunds(linkUpkeepID, 1);
assertEq(registry.getBalance(linkUpkeepID), startBalanceLINK + 1);
assertEq(registry.getBalance(usdUpkeepID), startBalanceUSDToken);
assertEq(linkToken.balanceOf(address(registry)), startLinkUpkeepBalance + 1);
assertEq(registry.getBalance(linkUpkeepID), startLinkUpkeepBalance + 1);
assertEq(registry.getBalance(usdUpkeepID), startUSDRegistryBalance);
assertEq(linkToken.balanceOf(address(registry)), startLINKRegistryBalance + 1);
assertEq(usdToken.balanceOf(address(registry)), startUSDUpkeepBalance);

registry.addFunds(usdUpkeepID, 2);
assertEq(registry.getBalance(linkUpkeepID), startBalanceLINK + 1);
assertEq(registry.getBalance(usdUpkeepID), startBalanceUSDToken + 2);
assertEq(linkToken.balanceOf(address(registry)), startLinkUpkeepBalance + 1);
assertEq(registry.getBalance(linkUpkeepID), startLinkUpkeepBalance + 1);
assertEq(registry.getBalance(usdUpkeepID), startUSDRegistryBalance + 2);
assertEq(linkToken.balanceOf(address(registry)), startLINKRegistryBalance + 1);
assertEq(usdToken.balanceOf(address(registry)), startUSDUpkeepBalance + 2);
}

Expand Down Expand Up @@ -1013,12 +1026,15 @@ contract NOPsSettlement is SetUp {
function _configureWithNewTransmitters(Registry registry, Registrar registrar) internal {
IERC20[] memory billingTokens = new IERC20[](1);
billingTokens[0] = IERC20(address(usdToken));

uint256[] memory minRegistrationFees = new uint256[](billingTokens.length);
minRegistrationFees[0] = 100000000000000000000; // 100 USD

address[] memory billingTokenAddresses = new address[](billingTokens.length);
for (uint256 i = 0; i < billingTokens.length; i++) {
billingTokenAddresses[i] = address(billingTokens[i]);
}

AutomationRegistryBase2_3.BillingConfig[]
memory billingTokenConfigs = new AutomationRegistryBase2_3.BillingConfig[](billingTokens.length);
billingTokenConfigs[0] = AutomationRegistryBase2_3.BillingConfig({
Expand All @@ -1029,9 +1045,9 @@ contract NOPsSettlement is SetUp {
minSpend: 1000000000000000000 // 1 USD
});

address[] memory registrars;
registrars = new address[](1);
address[] memory registrars = new address[](1);
registrars[0] = address(registrar);

AutomationRegistryBase2_3.OnchainConfig memory cfg = AutomationRegistryBase2_3.OnchainConfig({
checkGasLimit: 5_000_000,
stalenessSeconds: 90_000,
Expand All @@ -1050,6 +1066,7 @@ contract NOPsSettlement is SetUp {
reorgProtectionEnabled: true,
financeAdmin: FINANCE_ADMIN
});

registry.setConfigTypeSafe(
SIGNERS,
NEW_TRANSMITTERS,
Expand All @@ -1060,6 +1077,7 @@ contract NOPsSettlement is SetUp {
billingTokenAddresses,
billingTokenConfigs
);

registry.setPayees(NEW_PAYEES);
}
}
Expand Down Expand Up @@ -1379,6 +1397,7 @@ contract MigrateReceive is SetUp {
super.setUp();
(newRegistry, ) = deployAndConfigureRegistryAndRegistrar(AutoBase.PayoutMode.ON_CHAIN);
idsToMigrate.push(linkUpkeepID);
idsToMigrate.push(linkUpkeepID2);
idsToMigrate.push(usdUpkeepID);
idsToMigrate.push(nativeUpkeepID);
registry.setPeerRegistryMigrationPermission(address(newRegistry), 1);
Expand Down Expand Up @@ -1440,10 +1459,11 @@ contract MigrateReceive is SetUp {
registry.setUpkeepCheckData(nativeUpkeepID, randomBytes(25));

// record previous state
uint256[] memory prevUpkeepBalances = new uint256[](3);
uint256[] memory prevUpkeepBalances = new uint256[](4);
prevUpkeepBalances[0] = registry.getBalance(linkUpkeepID);
prevUpkeepBalances[1] = registry.getBalance(usdUpkeepID);
prevUpkeepBalances[2] = registry.getBalance(nativeUpkeepID);
prevUpkeepBalances[1] = registry.getBalance(linkUpkeepID2);
prevUpkeepBalances[2] = registry.getBalance(usdUpkeepID);
prevUpkeepBalances[3] = registry.getBalance(nativeUpkeepID);
uint256[] memory prevReserveBalances = new uint256[](3);
prevReserveBalances[0] = registry.getReserveAmount(address(linkToken));
prevReserveBalances[1] = registry.getReserveAmount(address(usdToken));
Expand All @@ -1452,42 +1472,53 @@ contract MigrateReceive is SetUp {
prevTokenBalances[0] = linkToken.balanceOf(address(registry));
prevTokenBalances[1] = usdToken.balanceOf(address(registry));
prevTokenBalances[2] = weth.balanceOf(address(registry));
bytes[] memory prevUpkeepData = new bytes[](3);
bytes[] memory prevUpkeepData = new bytes[](4);
prevUpkeepData[0] = abi.encode(registry.getUpkeep(linkUpkeepID));
prevUpkeepData[1] = abi.encode(registry.getUpkeep(usdUpkeepID));
prevUpkeepData[2] = abi.encode(registry.getUpkeep(nativeUpkeepID));
bytes[] memory prevUpkeepTriggerData = new bytes[](3);
prevUpkeepData[1] = abi.encode(registry.getUpkeep(linkUpkeepID2));
prevUpkeepData[2] = abi.encode(registry.getUpkeep(usdUpkeepID));
prevUpkeepData[3] = abi.encode(registry.getUpkeep(nativeUpkeepID));
bytes[] memory prevUpkeepTriggerData = new bytes[](4);
prevUpkeepTriggerData[0] = registry.getUpkeepTriggerConfig(linkUpkeepID);
prevUpkeepTriggerData[1] = registry.getUpkeepTriggerConfig(usdUpkeepID);
prevUpkeepTriggerData[2] = registry.getUpkeepTriggerConfig(nativeUpkeepID);
prevUpkeepTriggerData[1] = registry.getUpkeepTriggerConfig(linkUpkeepID2);
prevUpkeepTriggerData[2] = registry.getUpkeepTriggerConfig(usdUpkeepID);
prevUpkeepTriggerData[3] = registry.getUpkeepTriggerConfig(nativeUpkeepID);

// event expectations
vm.expectEmit(address(registry));
emit UpkeepMigrated(linkUpkeepID, prevUpkeepBalances[0], address(newRegistry));
vm.expectEmit(address(registry));
emit UpkeepMigrated(usdUpkeepID, prevUpkeepBalances[1], address(newRegistry));
emit UpkeepMigrated(linkUpkeepID2, prevUpkeepBalances[1], address(newRegistry));
vm.expectEmit(address(registry));
emit UpkeepMigrated(usdUpkeepID, prevUpkeepBalances[2], address(newRegistry));
vm.expectEmit(address(registry));
emit UpkeepMigrated(nativeUpkeepID, prevUpkeepBalances[2], address(newRegistry));
emit UpkeepMigrated(nativeUpkeepID, prevUpkeepBalances[3], address(newRegistry));
vm.expectEmit(address(newRegistry));
emit UpkeepReceived(linkUpkeepID, prevUpkeepBalances[0], address(registry));
vm.expectEmit(address(newRegistry));
emit UpkeepReceived(usdUpkeepID, prevUpkeepBalances[1], address(registry));
emit UpkeepReceived(linkUpkeepID2, prevUpkeepBalances[1], address(registry));
vm.expectEmit(address(newRegistry));
emit UpkeepReceived(nativeUpkeepID, prevUpkeepBalances[2], address(registry));
emit UpkeepReceived(usdUpkeepID, prevUpkeepBalances[2], address(registry));
vm.expectEmit(address(newRegistry));
emit UpkeepReceived(nativeUpkeepID, prevUpkeepBalances[3], address(registry));

// do the thing
registry.migrateUpkeeps(idsToMigrate, address(newRegistry));

// assert upkeep balances have been migrated
assertEq(registry.getBalance(linkUpkeepID), 0);
assertEq(registry.getBalance(linkUpkeepID2), 0);
assertEq(registry.getBalance(usdUpkeepID), 0);
assertEq(registry.getBalance(nativeUpkeepID), 0);
assertEq(newRegistry.getBalance(linkUpkeepID), prevUpkeepBalances[0]);
assertEq(newRegistry.getBalance(usdUpkeepID), prevUpkeepBalances[1]);
assertEq(newRegistry.getBalance(nativeUpkeepID), prevUpkeepBalances[2]);
assertEq(newRegistry.getBalance(linkUpkeepID2), prevUpkeepBalances[1]);
assertEq(newRegistry.getBalance(usdUpkeepID), prevUpkeepBalances[2]);
assertEq(newRegistry.getBalance(nativeUpkeepID), prevUpkeepBalances[3]);

// assert reserve balances have been adjusted
assertEq(newRegistry.getReserveAmount(address(linkToken)), newRegistry.getBalance(linkUpkeepID));
assertEq(
newRegistry.getReserveAmount(address(linkToken)),
newRegistry.getBalance(linkUpkeepID) + newRegistry.getBalance(linkUpkeepID2)
);
assertEq(newRegistry.getReserveAmount(address(usdToken)), newRegistry.getBalance(usdUpkeepID));
assertEq(newRegistry.getReserveAmount(address(weth)), newRegistry.getBalance(nativeUpkeepID));
assertEq(
Expand All @@ -1503,8 +1534,11 @@ contract MigrateReceive is SetUp {
prevReserveBalances[2] - registry.getReserveAmount(address(weth))
);

// assert token have been transfered
assertEq(linkToken.balanceOf(address(newRegistry)), newRegistry.getBalance(linkUpkeepID));
// assert token have been transferred
assertEq(
linkToken.balanceOf(address(newRegistry)),
newRegistry.getBalance(linkUpkeepID) + newRegistry.getBalance(linkUpkeepID2)
);
assertEq(usdToken.balanceOf(address(newRegistry)), newRegistry.getBalance(usdUpkeepID));
assertEq(weth.balanceOf(address(newRegistry)), newRegistry.getBalance(nativeUpkeepID));
assertEq(linkToken.balanceOf(address(registry)), prevTokenBalances[0] - linkToken.balanceOf(address(newRegistry)));
Expand All @@ -1513,11 +1547,13 @@ contract MigrateReceive is SetUp {

// assert upkeep data matches
assertEq(prevUpkeepData[0], abi.encode(newRegistry.getUpkeep(linkUpkeepID)));
assertEq(prevUpkeepData[1], abi.encode(newRegistry.getUpkeep(usdUpkeepID)));
assertEq(prevUpkeepData[2], abi.encode(newRegistry.getUpkeep(nativeUpkeepID)));
assertEq(prevUpkeepData[1], abi.encode(newRegistry.getUpkeep(linkUpkeepID2)));
assertEq(prevUpkeepData[2], abi.encode(newRegistry.getUpkeep(usdUpkeepID)));
assertEq(prevUpkeepData[3], abi.encode(newRegistry.getUpkeep(nativeUpkeepID)));
assertEq(prevUpkeepTriggerData[0], newRegistry.getUpkeepTriggerConfig(linkUpkeepID));
assertEq(prevUpkeepTriggerData[1], newRegistry.getUpkeepTriggerConfig(usdUpkeepID));
assertEq(prevUpkeepTriggerData[2], newRegistry.getUpkeepTriggerConfig(nativeUpkeepID));
assertEq(prevUpkeepTriggerData[1], newRegistry.getUpkeepTriggerConfig(linkUpkeepID2));
assertEq(prevUpkeepTriggerData[2], newRegistry.getUpkeepTriggerConfig(usdUpkeepID));
assertEq(prevUpkeepTriggerData[3], newRegistry.getUpkeepTriggerConfig(nativeUpkeepID));

vm.stopPrank();
}
Expand Down
35 changes: 22 additions & 13 deletions contracts/src/v0.8/automation/dev/v2_3/AutomationRegistrar2_3.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {IERC677Receiver} from "../../../shared/interfaces/IERC677Receiver.sol";
import {IERC20} from "../../../vendor/openzeppelin-solidity/v4.8.3/contracts/token/ERC20/IERC20.sol";
import {IWrappedNative} from "../interfaces/v2_3/IWrappedNative.sol";
import {SafeCast} from "../../../vendor/openzeppelin-solidity/v4.8.3/contracts/utils/math/SafeCast.sol";
import {SafeERC20} from "../../../vendor/openzeppelin-solidity/v4.8.3/contracts/token/ERC20/utils/SafeERC20.sol";

/**
* @notice Contract to accept requests for upkeep registrations
Expand All @@ -21,6 +22,8 @@ import {SafeCast} from "../../../vendor/openzeppelin-solidity/v4.8.3/contracts/u
* they can just listen to `RegistrationRequested` & `RegistrationApproved` events and know the status on registrations.
*/
contract AutomationRegistrar2_3 is TypeAndVersionInterface, ConfirmedOwner, IERC677Receiver {
using SafeERC20 for IERC20;

/**
* DISABLED: No auto approvals, all new upkeeps should be approved manually.
* ENABLED_SENDER_ALLOWLIST: Auto approvals for allowed senders subject to max allowed. Manual for rest.
Expand Down Expand Up @@ -74,6 +77,7 @@ contract AutomationRegistrar2_3 is TypeAndVersionInterface, ConfirmedOwner, IERC
struct PendingRequest {
address admin;
uint96 balance;
IERC20 billingToken;
}
/**
* @member upkeepContract address to perform upkeep on
Expand Down Expand Up @@ -145,6 +149,7 @@ contract AutomationRegistrar2_3 is TypeAndVersionInterface, ConfirmedOwner, IERC
error InvalidBillingToken();
error InvalidDataLength();
error TransferFailed(address to);
error DuplicateEntry();
error OnlyAdminOrOwner();
error OnlyLink();
error RequestNotFound();
Expand Down Expand Up @@ -190,9 +195,7 @@ contract AutomationRegistrar2_3 is TypeAndVersionInterface, ConfirmedOwner, IERC
i_WRAPPED_NATIVE_TOKEN.deposit{value: msg.value}();
} else {
// send ERC20 payment, including wrapped native token
if (!requestParams.billingToken.transferFrom(msg.sender, address(this), requestParams.amount)) {
revert TransferFailed(address(this));
}
requestParams.billingToken.safeTransferFrom(msg.sender, address(this), requestParams.amount);
}

return _register(requestParams, msg.sender);
Expand All @@ -217,22 +220,22 @@ contract AutomationRegistrar2_3 is TypeAndVersionInterface, ConfirmedOwner, IERC
}

/**
* @notice cancel will remove a registration request and return the refunds to the request.admin
* @notice cancel will remove a registration request from the pending request queue and return the refunds to the request.admin
* @param hash the request hash
*/
function cancel(bytes32 hash) external {
PendingRequest memory request = s_pendingRequests[hash];

if (!(msg.sender == request.admin || msg.sender == owner())) {
revert OnlyAdminOrOwner();
}
if (request.admin == address(0)) {
revert RequestNotFound();
}
delete s_pendingRequests[hash];
bool success = i_LINK.transfer(request.admin, request.balance);
if (!success) {
revert TransferFailed(request.admin);
}

request.billingToken.safeTransfer(request.admin, request.balance);

emit RegistrationRejected(hash);
}

Expand Down Expand Up @@ -340,9 +343,8 @@ contract AutomationRegistrar2_3 is TypeAndVersionInterface, ConfirmedOwner, IERC

/**
* @dev verify registration request and emit RegistrationRequested event
* @dev we currently allow multiple duplicate registrations by adding to the original registration's balance
* we could make this much simpler by using a nonce to differentiate otherwise identical requests and then
* we don't have to worry about identical registrations
* @dev we don't allow multiple duplicate registrations by adding to the original registration's balance
* users can cancel and re-register if they want to update the registration
*/
function _register(RegistrationParams memory params, address sender) private returns (uint256) {
if (params.amount < s_minRegistrationAmounts[params.billingToken]) {
Expand All @@ -356,6 +358,10 @@ contract AutomationRegistrar2_3 is TypeAndVersionInterface, ConfirmedOwner, IERC
}
bytes32 hash = keccak256(abi.encode(params));

if (s_pendingRequests[hash].admin != address(0)) {
revert DuplicateEntry();
}

emit RegistrationRequested(
hash,
params.name,
Expand All @@ -375,8 +381,11 @@ contract AutomationRegistrar2_3 is TypeAndVersionInterface, ConfirmedOwner, IERC
s_triggerRegistrations[params.triggerType].approvedCount++;
upkeepId = _approve(params, hash);
} else {
uint96 newBalance = s_pendingRequests[hash].balance + params.amount;
s_pendingRequests[hash] = PendingRequest({admin: params.adminAddress, balance: newBalance});
s_pendingRequests[hash] = PendingRequest({
admin: params.adminAddress,
balance: params.amount,
billingToken: params.billingToken
});
}

return upkeepId;
Expand Down
Loading

0 comments on commit 521a035

Please sign in to comment.