Merge branch 'develop' into another-flakeytest-runner-fix

.github/workflows/ci-core.yml

@@ -80,7 +80,7 @@ jobs:
         if: needs.init.outputs.on_trigger_lint == 'true'
         uses: golangci/golangci-lint-action@3a919529898de77ec3da873e3063ca4b10e7f5cc # v3.7.0
         with:
-          version: v1.53.3
+          version: v1.54.2
           only-new-issues: ${{ github.event.schedule == '' }} # show only new issues, unless it's a scheduled run
           args: --out-format checkstyle:golangci-lint-report.xml
       - name: Print lint report artifact

.tool-versions

@@ -4,5 +4,5 @@ nodejs 16.16.0
 postgres 13.3
 helm 3.10.3
 zig 0.10.1
-golangci-lint 1.53.3
+golangci-lint 1.54.2
 shellspec 0.28.1

GNUmakefile

@@ -146,7 +146,7 @@ config-docs: ## Generate core node configuration documentation
 .PHONY: golangci-lint
 golangci-lint: ## Run golangci-lint for all issues.
 	[ -d "./golangci-lint" ] || mkdir ./golangci-lint && \
-	docker run --rm -v $(shell pwd):/app -w /app golangci/golangci-lint:v1.53.2 golangci-lint run --max-issues-per-linter 0 --max-same-issues 0 > ./golangci-lint/$(shell date +%Y-%m-%d_%H:%M:%S).txt
+	docker run --rm -v $(shell pwd):/app -w /app golangci/golangci-lint:v1.54.2 golangci-lint run --max-issues-per-linter 0 --max-same-issues 0 > ./golangci-lint/$(shell date +%Y-%m-%d_%H:%M:%S).txt
 
 
 GORELEASER_CONFIG ?= .goreleaser.yaml

contracts/scripts/native_solc_compile_all_vrf

@@ -51,6 +51,7 @@ compileContract vrf/BatchBlockhashStore.sol
 compileContract vrf/BatchVRFCoordinatorV2.sol
 compileContract vrf/testhelpers/VRFCoordinatorV2TestHelper.sol
 compileContractAltOpts vrf/VRFCoordinatorV2.sol 10000
+compileContract mocks/VRFCoordinatorV2Mock.sol
 compileContract vrf/VRFOwner.sol
 
 # VRF V2Plus

contracts/src/v0.8/mocks/VRFCoordinatorV2Mock.sol

@@ -5,8 +5,9 @@ pragma solidity ^0.8.4;
 import "../shared/interfaces/LinkTokenInterface.sol";
 import "../interfaces/VRFCoordinatorV2Interface.sol";
 import "../vrf/VRFConsumerBaseV2.sol";
+import "../shared/access/ConfirmedOwner.sol";
 
-contract VRFCoordinatorV2Mock is VRFCoordinatorV2Interface {
+contract VRFCoordinatorV2Mock is VRFCoordinatorV2Interface, ConfirmedOwner {
   uint96 public immutable BASE_FEE;
   uint96 public immutable GAS_PRICE_LINK;
   uint16 public immutable MAX_CONSUMERS = 100;
@@ -17,6 +18,7 @@ contract VRFCoordinatorV2Mock is VRFCoordinatorV2Interface {
   error TooManyConsumers();
   error InvalidConsumer();
   error InvalidRandomWords();
+  error Reentrant();
 
   event RandomWordsRequested(
     bytes32 indexed keyHash,
@@ -34,7 +36,13 @@ contract VRFCoordinatorV2Mock is VRFCoordinatorV2Interface {
   event SubscriptionCanceled(uint64 indexed subId, address to, uint256 amount);
   event ConsumerAdded(uint64 indexed subId, address consumer);
   event ConsumerRemoved(uint64 indexed subId, address consumer);
+  event ConfigSet();
 
+  struct Config {
+    // Reentrancy protection.
+    bool reentrancyLock;
+  }
+  Config private s_config;
   uint64 s_currentSubId;
   uint256 s_nextRequestId = 1;
   uint256 s_nextPreSeed = 100;
@@ -52,9 +60,18 @@ contract VRFCoordinatorV2Mock is VRFCoordinatorV2Interface {
   }
   mapping(uint256 => Request) s_requests; /* requestId */ /* request */
 
-  constructor(uint96 _baseFee, uint96 _gasPriceLink) {
+  constructor(uint96 _baseFee, uint96 _gasPriceLink) ConfirmedOwner(msg.sender) {
     BASE_FEE = _baseFee;
     GAS_PRICE_LINK = _gasPriceLink;
+    setConfig();
+  }
+
+  /**
+   * @notice Sets the configuration of the vrfv2 mock coordinator
+   */
+  function setConfig() public onlyOwner {
+    s_config = Config({reentrancyLock: false});
+    emit ConfigSet();
   }
 
   function consumerIsAdded(uint64 _subId, address _consumer) public view returns (bool) {
@@ -85,7 +102,7 @@ contract VRFCoordinatorV2Mock is VRFCoordinatorV2Interface {
    * @param _requestId the request to fulfill
    * @param _consumer the VRF randomness consumer to send the result to
    */
-  function fulfillRandomWords(uint256 _requestId, address _consumer) external {
+  function fulfillRandomWords(uint256 _requestId, address _consumer) external nonReentrant {
     fulfillRandomWordsWithOverride(_requestId, _consumer, new uint256[](0));
   }
 
@@ -114,7 +131,9 @@ contract VRFCoordinatorV2Mock is VRFCoordinatorV2Interface {
 
     VRFConsumerBaseV2 v;
     bytes memory callReq = abi.encodeWithSelector(v.rawFulfillRandomWords.selector, _requestId, _words);
+    s_config.reentrancyLock = true;
     (bool success, ) = _consumer.call{gas: req.callbackGasLimit}(callReq);
+    s_config.reentrancyLock = false;
 
     uint96 payment = uint96(BASE_FEE + ((startGas - gasleft()) * GAS_PRICE_LINK));
     if (s_subscriptions[req.subId].balance < payment) {
@@ -146,7 +165,7 @@ contract VRFCoordinatorV2Mock is VRFCoordinatorV2Interface {
     uint16 _minimumRequestConfirmations,
     uint32 _callbackGasLimit,
     uint32 _numWords
-  ) external override onlyValidConsumer(_subId, msg.sender) returns (uint256) {
+  ) external override nonReentrant onlyValidConsumer(_subId, msg.sender) returns (uint256) {
     if (s_subscriptions[_subId].owner == address(0)) {
       revert InvalidSubscription();
     }
@@ -185,7 +204,7 @@ contract VRFCoordinatorV2Mock is VRFCoordinatorV2Interface {
     return (s_subscriptions[_subId].balance, 0, s_subscriptions[_subId].owner, s_consumers[_subId]);
   }
 
-  function cancelSubscription(uint64 _subId, address _to) external override onlySubOwner(_subId) {
+  function cancelSubscription(uint64 _subId, address _to) external override onlySubOwner(_subId) nonReentrant {
     emit SubscriptionCanceled(_subId, _to, s_subscriptions[_subId].balance);
     delete (s_subscriptions[_subId]);
   }
@@ -221,7 +240,7 @@ contract VRFCoordinatorV2Mock is VRFCoordinatorV2Interface {
   function removeConsumer(
     uint64 _subId,
     address _consumer
-  ) external override onlySubOwner(_subId) onlyValidConsumer(_subId, _consumer) {
+  ) external override onlySubOwner(_subId) onlyValidConsumer(_subId, _consumer) nonReentrant {
     address[] storage consumers = s_consumers[_subId];
     for (uint256 i = 0; i < consumers.length; i++) {
       if (consumers[i] == _consumer) {
@@ -276,6 +295,13 @@ contract VRFCoordinatorV2Mock is VRFCoordinatorV2Interface {
     );
   }
 
+  modifier nonReentrant() {
+    if (s_config.reentrancyLock) {
+      revert Reentrant();
+    }
+    _;
+  }
+
   function getFallbackWeiPerUnitLink() external view returns (int256) {
     return 4000000000000000; // 0.004 Ether
   }

contracts/test/v0.8/dev/VRFCoordinatorV2Mock.test.ts

@@ -262,7 +262,7 @@ describe('VRFCoordinatorV2Mock', () => {
       expect(receipt.events[0].args['success']).to.equal(true)
       assert(
         receipt.events[0].args['payment']
-          .sub(BigNumber.from('100119017000000000'))
+          .sub(BigNumber.from('100119403000000000'))
           .lt(BigNumber.from('10000000000')),
       )
 
@@ -315,7 +315,7 @@ describe('VRFCoordinatorV2Mock', () => {
       expect(receipt.events[0].args['success']).to.equal(true)
       assert(
         receipt.events[0].args['payment']
-          .sub(BigNumber.from('100119017000000000'))
+          .sub(BigNumber.from('100120516000000000'))
           .lt(BigNumber.from('10000000000')),
       )
 

contracts/test/v0.8/foundry/vrf/TrustedBlockhashStore.t.sol

@@ -48,6 +48,7 @@ contract TrustedBlockhashStoreTest is BaseTest {
     assertEq(blockhash(unreachableBlock), 0);
 
     // Store blockhash from whitelisted address;
+    uint256[] memory invalidBlockNums = new uint256[](0);
     uint256[] memory blockNums = new uint256[](1);
     blockNums[0] = unreachableBlock;
     bytes32[] memory blockhashes = new bytes32[](1);
@@ -64,9 +65,25 @@ contract TrustedBlockhashStoreTest is BaseTest {
     vm.expectRevert("Only callable by owner");
     bhs.setWhitelist(new address[](0));
 
-    // Should store unreachable blocks via whitelisted address.
+    // Should not store for a mismatched list of block numbers and hashes.
     changePrank(LINK_WHALE);
+    vm.expectRevert(TrustedBlockhashStore.InvalidTrustedBlockhashes.selector);
+    bhs.storeTrusted(invalidBlockNums, blockhashes, recentBlockNumber, blockhash(recentBlockNumber));
+
+    // Should store unreachable blocks via whitelisted address.
     bhs.storeTrusted(blockNums, blockhashes, recentBlockNumber, blockhash(recentBlockNumber));
     assertEq(bhs.getBlockhash(unreachableBlock), unreachableBlockhash);
+
+    // Change whitelist. Assert that the old whitelisted address can no longer store,
+    // but the new one can.
+    address[] memory newWhitelist = new address[](1);
+    newWhitelist[0] = LINK_WHALE_2;
+    bhs.setWhitelist(newWhitelist);
+
+    vm.expectRevert(TrustedBlockhashStore.NotInWhitelist.selector);
+    bhs.storeTrusted(blockNums, blockhashes, recentBlockNumber, blockhash(recentBlockNumber));
+
+    changePrank(LINK_WHALE_2);
+    bhs.storeTrusted(blockNums, blockhashes, recentBlockNumber, blockhash(recentBlockNumber));
   }
 }

contracts/test/v0.8/foundry/vrf/VRFV2Wrapper.t.sol

@@ -5,6 +5,7 @@ import {VRF} from "../../../../src/v0.8/vrf/VRF.sol";
 import {MockLinkToken} from "../../../../src/v0.8/mocks/MockLinkToken.sol";
 import {MockV3Aggregator} from "../../../../src/v0.8/tests/MockV3Aggregator.sol";
 import {ExposedVRFCoordinatorV2Plus} from "../../../../src/v0.8/dev/vrf/testhelpers/ExposedVRFCoordinatorV2Plus.sol";
+import {VRFV2PlusWrapperConsumerBase} from "../../../../src/v0.8/dev/vrf/VRFV2PlusWrapperConsumerBase.sol";
 import {VRFV2PlusWrapperConsumerExample} from "../../../../src/v0.8/dev/vrf/testhelpers/VRFV2PlusWrapperConsumerExample.sol";
 import {VRFCoordinatorV2Plus} from "../../../../src/v0.8/dev/vrf/VRFCoordinatorV2Plus.sol";
 import {VRFV2PlusWrapper} from "../../../../src/v0.8/dev/vrf/VRFV2PlusWrapper.sol";
@@ -70,6 +71,21 @@ contract VRFV2PlusWrapperTest is BaseTest {
       vrfKeyHash, // keyHash
       10 // max number of words
     );
+    (
+      ,
+      ,
+      ,
+      uint32 _wrapperGasOverhead,
+      uint32 _coordinatorGasOverhead,
+      uint8 _wrapperPremiumPercentage,
+      bytes32 _keyHash,
+      uint8 _maxNumWords
+    ) = s_wrapper.getConfig();
+    assertEq(_wrapperGasOverhead, wrapperGasOverhead);
+    assertEq(_coordinatorGasOverhead, coordinatorGasOverhead);
+    assertEq(0, _wrapperPremiumPercentage);
+    assertEq(vrfKeyHash, _keyHash);
+    assertEq(10, _maxNumWords);
   }
 
   event RandomWordsRequested(
@@ -84,11 +100,42 @@ contract VRFV2PlusWrapperTest is BaseTest {
     address indexed sender
   );
 
+  function testSetLinkAndLinkEthFeed() public {
+    VRFV2PlusWrapper wrapper = new VRFV2PlusWrapper(address(0), address(0), address(s_testCoordinator));
+
+    // Set LINK and LINK/ETH feed on wrapper.
+    wrapper.setLINK(address(s_linkToken));
+    wrapper.setLinkEthFeed(address(s_linkEthFeed));
+    assertEq(address(wrapper.s_link()), address(s_linkToken));
+    assertEq(address(wrapper.s_linkEthFeed()), address(s_linkEthFeed));
+
+    // Revert for subsequent assignment.
+    vm.expectRevert(VRFV2PlusWrapper.LinkAlreadySet.selector);
+    wrapper.setLINK(address(s_linkToken));
+
+    // Consumer can set LINK token.
+    VRFV2PlusWrapperConsumerExample consumer = new VRFV2PlusWrapperConsumerExample(address(0), address(wrapper));
+    consumer.setLinkToken(address(s_linkToken));
+
+    // Revert for subsequent assignment.
+    vm.expectRevert(VRFV2PlusWrapperConsumerBase.LINKAlreadySet.selector);
+    consumer.setLinkToken(address(s_linkToken));
+  }
+
   function testRequestAndFulfillRandomWordsNativeWrapper() public {
     // Fund subscription.
     s_testCoordinator.fundSubscriptionWithEth{value: 10 ether}(s_wrapper.SUBSCRIPTION_ID());
     vm.deal(address(s_consumer), 10 ether);
 
+    // Get type and version.
+    assertEq(s_wrapper.typeAndVersion(), "VRFV2Wrapper 1.0.0");
+
+    // Cannot make request while disabled.
+    s_wrapper.disable();
+    vm.expectRevert("wrapper is disabled");
+    s_consumer.makeRequestNative(500_000, 0, 1);
+    s_wrapper.enable();
+
     // Request randomness from wrapper.
     uint32 callbackGasLimit = 1_000_000;
     vm.expectEmit(true, true, true, true);
@@ -114,7 +161,11 @@ contract VRFV2PlusWrapperTest is BaseTest {
 
     (uint256 paid, bool fulfilled, bool native) = s_consumer.s_requests(requestId);
     uint32 expectedPaid = callbackGasLimit + wrapperGasOverhead + coordinatorGasOverhead;
+    uint256 wrapperNativeCostEstimate = s_wrapper.estimateRequestPriceNative(callbackGasLimit, tx.gasprice);
+    uint256 wrapperCostCalculation = s_wrapper.calculateRequestPriceNative(callbackGasLimit);
     assertEq(paid, expectedPaid);
+    assertEq(uint256(paid), wrapperNativeCostEstimate);
+    assertEq(wrapperNativeCostEstimate, wrapperCostCalculation);
     assertEq(fulfilled, false);
     assertEq(native, true);
     assertEq(address(s_consumer).balance, 10 ether - expectedPaid);
@@ -129,6 +180,13 @@ contract VRFV2PlusWrapperTest is BaseTest {
     (, bool nowFulfilled, uint256[] memory storedWords) = s_consumer.getRequestStatus(requestId);
     assertEq(nowFulfilled, true);
     assertEq(storedWords[0], 123);
+
+    // Withdraw funds from wrapper.
+    changePrank(LINK_WHALE);
+    uint256 priorWhaleBalance = LINK_WHALE.balance;
+    s_wrapper.withdrawNative(LINK_WHALE, paid);
+    assertEq(LINK_WHALE.balance, priorWhaleBalance + paid);
+    assertEq(address(s_wrapper).balance, 0);
   }
 
   function testRequestAndFulfillRandomWordsLINKWrapper() public {
@@ -162,7 +220,11 @@ contract VRFV2PlusWrapperTest is BaseTest {
     // Assert that the request was made correctly.
     (uint256 paid, bool fulfilled, bool native) = s_consumer.s_requests(requestId);
     uint32 expectedPaid = (callbackGasLimit + wrapperGasOverhead + coordinatorGasOverhead) * 2;
+    uint256 wrapperCostEstimate = s_wrapper.estimateRequestPrice(callbackGasLimit, tx.gasprice);
+    uint256 wrapperCostCalculation = s_wrapper.calculateRequestPrice(callbackGasLimit);
     assertEq(paid, expectedPaid); // 1_030_000 * 2 for link/eth ratio
+    assertEq(uint256(paid), wrapperCostEstimate);
+    assertEq(wrapperCostEstimate, wrapperCostCalculation);
     assertEq(fulfilled, false);
     assertEq(native, false);
     assertEq(s_linkToken.balanceOf(address(s_consumer)), 10 ether - expectedPaid);
@@ -177,5 +239,12 @@ contract VRFV2PlusWrapperTest is BaseTest {
     (, bool nowFulfilled, uint256[] memory storedWords) = s_consumer.getRequestStatus(requestId);
     assertEq(nowFulfilled, true);
     assertEq(storedWords[0], 456);
+
+    // Withdraw funds from wrapper.
+    changePrank(LINK_WHALE);
+    uint256 priorWhaleBalance = s_linkToken.balanceOf(LINK_WHALE);
+    s_wrapper.withdraw(LINK_WHALE, paid);
+    assertEq(s_linkToken.balanceOf(LINK_WHALE), priorWhaleBalance + paid);
+    assertEq(s_linkToken.balanceOf(address(s_wrapper)), 0);
   }
 }

core/chains/chain_kv.go

@@ -17,7 +17,6 @@ type ChainsKV[T types.ChainService] struct {
 var ErrNoSuchChainID = errors.New("chain id does not exist")
 
 func NewChainsKV[T types.ChainService](cs map[string]T) *ChainsKV[T] {
-
 	return &ChainsKV[T]{
 		chains: cs,
 	}

core/chains/chain_kv_test.go

@@ -89,19 +89,13 @@ func (s *testChainService) HealthReport() map[string]error {
 	return map[string]error{}
 }
 
-// Implement updated [loop.Relay] interface funcs in preparation for BCF-2441
-// TODO update this comment after BCF-2441 is done
+// Implement [types.ChainService] interface
 func (s *testChainService) GetChainStatus(ctx context.Context) (stat types.ChainStatus, err error) {
 	return
 }
 func (s *testChainService) ListNodeStatuses(ctx context.Context, pageSize int32, pageToken string) (stats []types.NodeStatus, nextPageToken string, total int, err error) {
 	return
 }
-
 func (s *testChainService) Transact(ctx context.Context, from string, to string, amount *big.Int, balanceCheck bool) error {
 	return nil
 }
-
-func (s *testChainService) SendTx(ctx context.Context, from string, to string, amount *big.Int, balanceCheck bool) error {
-	return nil
-}

core/chains/config.go

@@ -2,9 +2,6 @@ package chains
 
 import (
 	"errors"
-
-	"github.com/smartcontractkit/chainlink-relay/pkg/logger"
-	"github.com/smartcontractkit/chainlink-relay/pkg/types"
 )
 
 var (
@@ -13,26 +10,7 @@ var (
 	ErrNotFound     = errors.New("not found")
 )
 
-type ChainConfigs interface {
-	Chains(offset, limit int, ids ...string) ([]types.ChainStatus, int, error)
-}
-
-type NodeConfigs[I ID, N Node] interface {
-	Node(name string) (N, error)
-	Nodes(chainID I) (nodes []N, err error)
-
-	NodeStatus(name string) (types.NodeStatus, error)
-}
-
-// Configs holds chain and node configurations.
-// TODO: BCF-2605 audit the usage of this interface and potentially remove it
-type Configs[I ID, N Node] interface {
-	ChainConfigs
-	NodeConfigs[I, N]
-}
-
 // ChainOpts holds options for configuring a Chain
-type ChainOpts[I ID, N Node] interface {
+type ChainOpts interface {
 	Validate() error
-	ConfigsAndLogger() (Configs[I, N], logger.Logger)
 }