Skip to content

Commit

Permalink
Merge branch 'develop' into feat/FUN-1332_allowlist_optimization
Browse files Browse the repository at this point in the history
  • Loading branch information
agparadiso committed May 6, 2024
2 parents 13a3a16 + 36cc95f commit f440ada
Show file tree
Hide file tree
Showing 31 changed files with 4,550 additions and 23 deletions.
5 changes: 5 additions & 0 deletions .changeset/grumpy-birds-serve.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"chainlink": patch
---

withdraw in offchain mode #bugfix
5 changes: 5 additions & 0 deletions .changeset/modern-trainers-hear.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"chainlink": patch
---

#internal Generate gethwrappers for capability registry changes
5 changes: 5 additions & 0 deletions .changeset/ninety-students-return.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"chainlink": patch
---

#updated Add gethwrappers for operatorforwarder contracts
5 changes: 5 additions & 0 deletions .changeset/tall-hats-brake.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"chainlink": patch
---

get available erc20 for payment #bugfix
5 changes: 5 additions & 0 deletions contracts/.changeset/calm-maps-vanish.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@chainlink/contracts": patch
---

withdraw in offchain mode #bugfix
5 changes: 5 additions & 0 deletions contracts/.changeset/proud-tables-grab.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@chainlink/contracts": patch
---

get available erc20s for payment #bugfix
5 changes: 5 additions & 0 deletions contracts/.changeset/three-hotels-agree.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@chainlink/contracts": patch
---

Add function to update nodes in capability registry

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,7 @@ contract AddFunds is SetUp {
registry.addFunds(nativeUpkeepID, 1);
assertEq(registry.getBalance(nativeUpkeepID), startRegistryBalance + 1);
assertEq(weth.balanceOf(address(registry)), startTokenBalance + 1);
assertEq(registry.getAvailableERC20ForPayment(address(weth)), 0);
}

// when msg.value is not 0, it uses the native payment path
Expand All @@ -214,6 +215,7 @@ contract AddFunds is SetUp {
registry.addFunds{value: 1}(nativeUpkeepID, 1000); // parameter amount should be ignored
assertEq(registry.getBalance(nativeUpkeepID), startRegistryBalance + 1);
assertEq(weth.balanceOf(address(registry)), startTokenBalance + 1);
assertEq(registry.getAvailableERC20ForPayment(address(weth)), 0);
}

// it fails when the billing token is not native, but trying to pay with native
Expand Down Expand Up @@ -345,9 +347,14 @@ contract Withdraw is SetUp {
registry.withdrawLink(FINANCE_ADMIN, 1); // but using link withdraw functions succeeds
}

// default is ON_CHAIN mode
function test_WithdrawERC20Fees_RevertsWhen_LinkAvailableForPaymentIsNegative() public {
_transmit(usdUpkeepID18, registry); // adds USD token to finance withdrawable, and gives NOPs a LINK balance
require(registry.linkAvailableForPayment() < 0, "linkAvailableForPayment should be negative");
require(
registry.getAvailableERC20ForPayment(address(usdToken18)) > 0,
"ERC20AvailableForPayment should be positive"
);
vm.expectRevert(Registry.InsufficientLinkLiquidity.selector);
vm.prank(FINANCE_ADMIN);
registry.withdrawERC20Fees(address(usdToken18), FINANCE_ADMIN, 1); // should revert
Expand All @@ -356,6 +363,27 @@ contract Withdraw is SetUp {
registry.withdrawERC20Fees(address(usdToken18), FINANCE_ADMIN, 1); // now finance can withdraw
}

function test_WithdrawERC20Fees_InOffChainMode_Happy() public {
// deploy and configure a registry with OFF_CHAIN payout
(Registry registry, ) = deployAndConfigureRegistryAndRegistrar(AutoBase.PayoutMode.OFF_CHAIN);

// register an upkeep and add funds
uint256 id = registry.registerUpkeep(address(TARGET1), 1000000, UPKEEP_ADMIN, 0, address(usdToken18), "", "", "");
_mintERC20_18Decimals(UPKEEP_ADMIN, 1e20);
vm.startPrank(UPKEEP_ADMIN);
usdToken18.approve(address(registry), 1e20);
registry.addFunds(id, 1e20);

// manually create a transmit so transmitters earn some rewards
_transmit(id, registry);
require(registry.linkAvailableForPayment() < 0, "linkAvailableForPayment should be negative");
vm.prank(FINANCE_ADMIN);
registry.withdrawERC20Fees(address(usdToken18), aMockAddress, 1); // finance can withdraw

// recipient should get the funds
assertEq(usdToken18.balanceOf(address(aMockAddress)), 1);
}

function testWithdrawERC20FeeSuccess() public {
// deposit excess USDToken to the registry (this goes to the "finance withdrawable" pool be default)
uint256 startReserveAmount = registry.getReserveAmount(address(usdToken18));
Expand Down Expand Up @@ -959,7 +987,8 @@ contract NOPsSettlement is SetUp {
registry.settleNOPsOffchain();
}

function testSettleNOPsOffchainSuccessTransmitterBalanceZeroed() public {
// 1. transmitter balance zeroed after settlement, 2. admin can withdraw ERC20, 3. switch to onchain mode, 4. link amount owed to NOPs stays the same
function testSettleNOPsOffchainSuccessWithERC20MultiSteps() public {
// deploy and configure a registry with OFF_CHAIN payout
(Registry registry, ) = deployAndConfigureRegistryAndRegistrar(AutoBase.PayoutMode.OFF_CHAIN);

Expand Down Expand Up @@ -1001,6 +1030,42 @@ contract NOPsSettlement is SetUp {

// after the offchain settlement, the total reserve amount of LINK should be 0
assertEq(registry.getReserveAmount(address(linkToken)), 0);
// should have some ERC20s in registry after transmit
uint256 erc20ForPayment1 = registry.getAvailableERC20ForPayment(address(usdToken18));
require(erc20ForPayment1 > 0, "ERC20AvailableForPayment should be positive");

vm.startPrank(UPKEEP_ADMIN);
vm.roll(100 + block.number);
// manually create a transmit so transmitters earn some rewards
_transmit(id, registry);

uint256 erc20ForPayment2 = registry.getAvailableERC20ForPayment(address(usdToken18));
require(erc20ForPayment2 > erc20ForPayment1, "ERC20AvailableForPayment should be greater after another transmit");

// finance admin comes to withdraw all available ERC20s
vm.startPrank(FINANCE_ADMIN);
registry.withdrawERC20Fees(address(usdToken18), FINANCE_ADMIN, erc20ForPayment2);

uint256 erc20ForPayment3 = registry.getAvailableERC20ForPayment(address(usdToken18));
require(erc20ForPayment3 == 0, "ERC20AvailableForPayment should be 0 now after withdrawal");

uint256 reservedLink = registry.getReserveAmount(address(linkToken));
require(reservedLink > 0, "Reserve amount of LINK should be positive since there was another transmit");

// owner comes to disable offchain mode
vm.startPrank(registry.owner());
registry.disableOffchainPayments();

// finance admin comes to withdraw all available ERC20s, should revert bc of insufficient link liquidity
vm.startPrank(FINANCE_ADMIN);
uint256 erc20ForPayment4 = registry.getAvailableERC20ForPayment(address(usdToken18));
vm.expectRevert(abi.encodeWithSelector(Registry.InsufficientLinkLiquidity.selector));
registry.withdrawERC20Fees(address(usdToken18), FINANCE_ADMIN, erc20ForPayment4);

// reserved link amount to NOPs should stay the same after switching to onchain mode
assertEq(registry.getReserveAmount(address(linkToken)), reservedLink);
// available ERC20 for payment should be 0 since finance admin withdrew all already
assertEq(erc20ForPayment4, 0);
}

function testSettleNOPsOffchainForDeactivatedTransmittersSuccess() public {
Expand Down Expand Up @@ -1563,8 +1628,21 @@ contract Transmit is SetUp {
upkeepIDs[0] = linkUpkeepID;
upkeepIDs[1] = usdUpkeepID18;
upkeepIDs[2] = nativeUpkeepID;

// withdraw-able by finance team should be 0
require(registry.getAvailableERC20ForPayment(address(usdToken18)) == 0, "ERC20AvailableForPayment should be 0");
require(registry.getAvailableERC20ForPayment(address(weth)) == 0, "ERC20AvailableForPayment should be 0");

// do the thing
_transmit(upkeepIDs, registry);

// withdraw-able by the finance team should be positive
require(
registry.getAvailableERC20ForPayment(address(usdToken18)) > 0,
"ERC20AvailableForPayment should be positive"
);
require(registry.getAvailableERC20ForPayment(address(weth)) > 0, "ERC20AvailableForPayment should be positive");

// assert upkeep balances have decreased
require(prevUpkeepBalances[0] > registry.getBalance(linkUpkeepID), "link upkeep balance should have decreased");
require(prevUpkeepBalances[1] > registry.getBalance(usdUpkeepID18), "usd upkeep balance should have decreased");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -430,14 +430,14 @@ contract AutomationRegistryLogicB2_3 is AutomationRegistryBase2_3, Chainable {
* @param asset the asset to withdraw
* @param to the address to send the fees to
* @param amount the amount to withdraw
* @dev we prevent withdrawing non-LINK fees unless there is sufficient LINK liquidity
* @dev in ON_CHAIN mode, we prevent withdrawing non-LINK fees unless there is sufficient LINK liquidity
* to cover all outstanding debts on the registry
*/
function withdrawERC20Fees(IERC20 asset, address to, uint256 amount) external {
_onlyFinanceAdminAllowed();
if (to == ZERO_ADDRESS) revert InvalidRecipient();
if (address(asset) == address(i_link)) revert InvalidToken();
if (_linkAvailableForPayment() < 0) revert InsufficientLinkLiquidity();
if (_linkAvailableForPayment() < 0 && s_payoutMode == PayoutMode.ON_CHAIN) revert InsufficientLinkLiquidity();
uint256 available = asset.balanceOf(address(this)) - s_reserveAmounts[asset];
if (amount > available) revert InsufficientBalance(available, amount);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -584,6 +584,13 @@ contract AutomationRegistryLogicC2_3 is AutomationRegistryBase2_3 {
return s_reserveAmounts[billingToken];
}

/**
* @notice returns the amount of a particular token that is withdraw-able by finance admin
*/
function getAvailableERC20ForPayment(IERC20 billingToken) external view returns (uint256) {
return billingToken.balanceOf(address(this)) - s_reserveAmounts[IERC20(address(billingToken))];
}

/**
* @notice returns the size of the LINK liquidity pool
*/
Expand Down
46 changes: 45 additions & 1 deletion contracts/src/v0.8/keystone/CapabilityRegistry.sol
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,16 @@ contract CapabilityRegistry is OwnerIsCreator, TypeAndVersionInterface {
/// @param nodeOperatorId The ID of the node operator that manages this node
event NodeAdded(bytes32 p2pId, uint256 nodeOperatorId);

/// @notice This event is emitted when a node is updated
/// @param p2pId The P2P ID of the node
/// @param nodeOperatorId The ID of the node operator that manages this node
/// @param signer The node's signer address
event NodeUpdated(bytes32 p2pId, uint256 nodeOperatorId, address signer);

/// @notice This error is thrown when trying to set the node's
/// signer address to zero
error InvalidNodeSigner();

/// @notice This error is thrown when trying add a capability that already
/// exists.
error CapabilityAlreadyExists();
Expand Down Expand Up @@ -236,12 +246,16 @@ contract CapabilityRegistry is OwnerIsCreator, TypeAndVersionInterface {
for (uint256 i; i < nodes.length; ++i) {
Node memory node = nodes[i];

bool isOwner = msg.sender == owner();

NodeOperator memory nodeOperator = s_nodeOperators[node.nodeOperatorId];
if (msg.sender != nodeOperator.admin) revert AccessForbidden();
if (!isOwner && msg.sender != nodeOperator.admin) revert AccessForbidden();

bool nodeExists = s_nodes[node.p2pId].supportedHashedCapabilityIds.length > 0;
if (nodeExists || bytes32(node.p2pId) == bytes32("")) revert InvalidNodeP2PId(node.p2pId);

if (node.signer == address(0)) revert InvalidNodeSigner();

if (node.supportedHashedCapabilityIds.length == 0)
revert InvalidNodeCapabilities(node.supportedHashedCapabilityIds);

Expand All @@ -255,6 +269,36 @@ contract CapabilityRegistry is OwnerIsCreator, TypeAndVersionInterface {
}
}

/// @notice Updates nodes. The node admin can update the node's signer address
/// and reconfigure its supported capabilities
/// @param nodes The nodes to update
function updateNodes(Node[] calldata nodes) external {
for (uint256 i; i < nodes.length; ++i) {
Node memory node = nodes[i];

bool isOwner = msg.sender == owner();

NodeOperator memory nodeOperator = s_nodeOperators[node.nodeOperatorId];
if (!isOwner && msg.sender != nodeOperator.admin) revert AccessForbidden();

bool nodeExists = s_nodes[node.p2pId].supportedHashedCapabilityIds.length > 0;
if (!nodeExists) revert InvalidNodeP2PId(node.p2pId);

if (node.signer == address(0)) revert InvalidNodeSigner();

if (node.supportedHashedCapabilityIds.length == 0)
revert InvalidNodeCapabilities(node.supportedHashedCapabilityIds);

for (uint256 j; j < node.supportedHashedCapabilityIds.length; ++j) {
if (!s_hashedCapabilityIds.contains(node.supportedHashedCapabilityIds[j]))
revert InvalidNodeCapabilities(node.supportedHashedCapabilityIds);
}

s_nodes[node.p2pId] = node;
emit NodeUpdated(node.p2pId, node.nodeOperatorId, node.signer);
}
}

/// @notice Gets a node's data
/// @param p2pId The P2P ID of the node to query for
/// @return Node The node data
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ contract CapabilityRegistry_AddNodesTest is BaseTest {
s_capabilityRegistry.addCapability(s_capabilityWithConfigurationContract);
}

function test_RevertWhen_CalledByNonNodeOperatorAdmin() public {
function test_RevertWhen_CalledByNonNodeOperatorAdminAndNonOwner() public {
changePrank(STRANGER);
CapabilityRegistry.Node[] memory nodes = new CapabilityRegistry.Node[](1);

Expand All @@ -36,6 +36,24 @@ contract CapabilityRegistry_AddNodesTest is BaseTest {
s_capabilityRegistry.addNodes(nodes);
}

function test_RevertWhen_SignerAddressEmpty() public {
changePrank(NODE_OPERATOR_ONE_ADMIN);
CapabilityRegistry.Node[] memory nodes = new CapabilityRegistry.Node[](1);

bytes32[] memory hashedCapabilityIds = new bytes32[](1);
hashedCapabilityIds[0] = s_basicHashedCapabilityId;

nodes[0] = CapabilityRegistry.Node({
nodeOperatorId: TEST_NODE_OPERATOR_ONE_ID,
p2pId: P2P_ID,
signer: address(0),
supportedHashedCapabilityIds: hashedCapabilityIds
});

vm.expectRevert(abi.encodeWithSelector(CapabilityRegistry.InvalidNodeSigner.selector));
s_capabilityRegistry.addNodes(nodes);
}

function test_RevertWhen_AddingDuplicateP2PId() public {
changePrank(NODE_OPERATOR_ONE_ADMIN);
CapabilityRegistry.Node[] memory nodes = new CapabilityRegistry.Node[](1);
Expand Down Expand Up @@ -133,4 +151,31 @@ contract CapabilityRegistry_AddNodesTest is BaseTest {
assertEq(node.supportedHashedCapabilityIds[0], s_basicHashedCapabilityId);
assertEq(node.supportedHashedCapabilityIds[1], s_capabilityWithConfigurationContractId);
}

function test_OwnerCanAddNodes() public {
changePrank(ADMIN);

CapabilityRegistry.Node[] memory nodes = new CapabilityRegistry.Node[](1);
bytes32[] memory hashedCapabilityIds = new bytes32[](2);
hashedCapabilityIds[0] = s_basicHashedCapabilityId;
hashedCapabilityIds[1] = s_capabilityWithConfigurationContractId;

nodes[0] = CapabilityRegistry.Node({
nodeOperatorId: TEST_NODE_OPERATOR_ONE_ID,
p2pId: P2P_ID,
signer: NODE_OPERATOR_ONE_SIGNER_ADDRESS,
supportedHashedCapabilityIds: hashedCapabilityIds
});

vm.expectEmit(address(s_capabilityRegistry));
emit NodeAdded(P2P_ID, TEST_NODE_OPERATOR_ONE_ID);
s_capabilityRegistry.addNodes(nodes);

CapabilityRegistry.Node memory node = s_capabilityRegistry.getNode(P2P_ID);
assertEq(node.nodeOperatorId, TEST_NODE_OPERATOR_ONE_ID);
assertEq(node.p2pId, P2P_ID);
assertEq(node.supportedHashedCapabilityIds.length, 2);
assertEq(node.supportedHashedCapabilityIds[0], s_basicHashedCapabilityId);
assertEq(node.supportedHashedCapabilityIds[1], s_capabilityWithConfigurationContractId);
}
}

0 comments on commit f440ada

Please sign in to comment.