Skip to content

Commit

Permalink
[VRF-592] Add LINK token check (#10422)
Browse files Browse the repository at this point in the history
* [VRF-592] Add LINK token check

* Add tests

* Update snapshot

* Add gethwrappers
  • Loading branch information
vreff authored Sep 1, 2023
1 parent 0ab6fef commit 748538f
Show file tree
Hide file tree
Showing 7 changed files with 148 additions and 18 deletions.
24 changes: 13 additions & 11 deletions contracts/gas-snapshots/vrf.gas-snapshot
Original file line number Diff line number Diff line change
@@ -1,16 +1,18 @@
TrustedBlockhashStoreTest:testGenericBHSFunctions() (gas: 53507)
TrustedBlockhashStoreTest:testTrustedBHSFunctions() (gas: 49536)
VRFCoordinatorV2Plus_Migration:testDeregister() (gas: 101083)
VRFCoordinatorV2Plus_Migration:testMigrateRevertsWhenInvalidCaller() (gas: 29377)
VRFCoordinatorV2Plus_Migration:testMigrateRevertsWhenInvalidCoordinator() (gas: 19877)
VRFCoordinatorV2Plus_Migration:testMigrateRevertsWhenPendingFulfillment() (gas: 237679)
VRFCoordinatorV2Plus_Migration:testMigrateRevertsWhenReentrant() (gas: 361358)
VRFCoordinatorV2Plus_Migration:testMigration() (gas: 471429)
VRFV2Plus:testCreateSubscription() (gas: 181121)
VRFV2Plus:testGetActiveSubscriptionIds() (gas: 3465607)
VRFV2Plus:testRegisterProvingKey() (gas: 101019)
VRFV2Plus:testRequestAndFulfillRandomWordsLINK() (gas: 755125)
VRFV2Plus:testRequestAndFulfillRandomWordsNative() (gas: 705549)
VRFV2Plus:testSetConfig() (gas: 73057)
VRFCoordinatorV2Plus_Migration:testMigrateRevertsWhenInvalidCaller() (gas: 29421)
VRFCoordinatorV2Plus_Migration:testMigrateRevertsWhenInvalidCoordinator() (gas: 19855)
VRFCoordinatorV2Plus_Migration:testMigrateRevertsWhenPendingFulfillment() (gas: 237702)
VRFCoordinatorV2Plus_Migration:testMigrateRevertsWhenReentrant() (gas: 354749)
VRFCoordinatorV2Plus_Migration:testMigration() (gas: 471631)
VRFCoordinatorV2Plus_Migration:testMigrationNoLink() (gas: 431052)
VRFV2Plus:testCancelSubWithNoLink() (gas: 160261)
VRFV2Plus:testCreateSubscription() (gas: 181127)
VRFV2Plus:testGetActiveSubscriptionIds() (gas: 3453659)
VRFV2Plus:testRegisterProvingKey() (gas: 101025)
VRFV2Plus:testRequestAndFulfillRandomWordsLINK() (gas: 755178)
VRFV2Plus:testRequestAndFulfillRandomWordsNative() (gas: 705581)
VRFV2Plus:testSetConfig() (gas: 73032)
VRFV2PlusWrapperTest:testRequestAndFulfillRandomWordsLINKWrapper() (gas: 390063)
VRFV2PlusWrapperTest:testRequestAndFulfillRandomWordsNativeWrapper() (gas: 290612)
9 changes: 7 additions & 2 deletions contracts/src/v0.8/dev/vrf/SubscriptionAPI.sol
Original file line number Diff line number Diff line change
Expand Up @@ -400,9 +400,14 @@ abstract contract SubscriptionAPI is ConfirmedOwner, IERC677Receiver, IVRFSubscr

function cancelSubscriptionHelper(uint256 subId, address to) internal {
(uint96 balance, uint96 ethBalance) = deleteSubscription(subId);
if (!LINK.transfer(to, uint256(balance))) {
revert InsufficientBalance();

// Only withdraw LINK if the token is active and there is a balance.
if (address(LINK) != address(0) && balance != 0) {
if (!LINK.transfer(to, uint256(balance))) {
revert InsufficientBalance();
}
}

// send eth to the "to" address using call
(bool success, ) = to.call{value: uint256(ethBalance)}("");
if (!success) {
Expand Down
6 changes: 5 additions & 1 deletion contracts/src/v0.8/dev/vrf/VRFCoordinatorV2Plus.sol
Original file line number Diff line number Diff line change
Expand Up @@ -672,7 +672,11 @@ contract VRFCoordinatorV2Plus is VRF, SubscriptionAPI {
bytes memory encodedData = abi.encode(migrationData);
deleteSubscription(subId);
IVRFCoordinatorV2PlusMigration(newCoordinator).onMigration{value: ethBalance}(encodedData);
require(LINK.transfer(address(newCoordinator), balance), "insufficient funds");

// Only transfer LINK if the token is active and there is a balance.
if (address(LINK) != address(0) && balance != 0) {
require(LINK.transfer(address(newCoordinator), balance), "insufficient funds");
}

// despite the fact that we follow best practices this is still probably safest
// to prevent any re-entrancy possibilities.
Expand Down
104 changes: 104 additions & 0 deletions contracts/test/v0.8/foundry/vrf/VRFCoordinatorV2Plus_Migration.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,19 @@ contract VRFCoordinatorV2Plus_Migration is BaseTest {

ExposedVRFCoordinatorV2Plus v1Coordinator;
VRFCoordinatorV2Plus_V2Example v2Coordinator;
ExposedVRFCoordinatorV2Plus v1Coordinator_noLink;
VRFCoordinatorV2Plus_V2Example v2Coordinator_noLink;
uint256 subId;
uint256 subId_noLink;
VRFV2PlusConsumerExample testConsumer;
VRFV2PlusConsumerExample testConsumer_noLink;
MockLinkToken linkToken;
address linkTokenAddr;
MockV3Aggregator linkEthFeed;
address v1CoordinatorAddr;
address v2CoordinatorAddr;
address v1CoordinatorAddr_noLink;
address v2CoordinatorAddr_noLink;

event CoordinatorRegistered(address coordinatorAddress);
event CoordinatorDeregistered(address coordinatorAddress);
Expand All @@ -43,14 +49,19 @@ contract VRFCoordinatorV2Plus_Migration is BaseTest {
vm.deal(OWNER, 100 ether);
address bhs = makeAddr("bhs");
v1Coordinator = new ExposedVRFCoordinatorV2Plus(bhs);
v1Coordinator_noLink = new ExposedVRFCoordinatorV2Plus(bhs);
subId = v1Coordinator.createSubscription();
subId_noLink = v1Coordinator_noLink.createSubscription();
linkToken = new MockLinkToken();
linkEthFeed = new MockV3Aggregator(18, 500000000000000000); // .5 ETH (good for testing)
v1Coordinator.setLINKAndLINKETHFeed(address(linkToken), address(linkEthFeed));
linkTokenAddr = address(linkToken);
v2Coordinator = new VRFCoordinatorV2Plus_V2Example(address(linkToken), address(v1Coordinator));
v2Coordinator_noLink = new VRFCoordinatorV2Plus_V2Example(address(0), address(v1Coordinator_noLink));
v1CoordinatorAddr = address(v1Coordinator);
v2CoordinatorAddr = address(v2Coordinator);
v1CoordinatorAddr_noLink = address(v1Coordinator_noLink);
v2CoordinatorAddr_noLink = address(v2Coordinator_noLink);

vm.expectEmit(
false, // no first indexed topic
Expand All @@ -62,7 +73,18 @@ contract VRFCoordinatorV2Plus_Migration is BaseTest {
v1Coordinator.registerMigratableCoordinator(v2CoordinatorAddr);
assertTrue(v1Coordinator.isTargetRegisteredExternal(v2CoordinatorAddr));

vm.expectEmit(
false, // no first indexed topic
false, // no second indexed topic
false, // no third indexed topic
true // check data (target coordinator address)
);
emit CoordinatorRegistered(v2CoordinatorAddr_noLink);
v1Coordinator_noLink.registerMigratableCoordinator(v2CoordinatorAddr_noLink);
assertTrue(v1Coordinator_noLink.isTargetRegisteredExternal(v2CoordinatorAddr_noLink));

testConsumer = new VRFV2PlusConsumerExample(address(v1Coordinator), address(linkToken));
testConsumer_noLink = new VRFV2PlusConsumerExample(address(v1Coordinator_noLink), address(0));
v1Coordinator.setConfig(
DEFAULT_REQUEST_CONFIRMATIONS,
DEFAULT_CALLBACK_GAS_LIMIT,
Expand All @@ -71,8 +93,17 @@ contract VRFCoordinatorV2Plus_Migration is BaseTest {
20_000,
VRFCoordinatorV2Plus.FeeConfig({fulfillmentFlatFeeLinkPPM: 200, fulfillmentFlatFeeEthPPM: 100})
);
v1Coordinator_noLink.setConfig(
DEFAULT_REQUEST_CONFIRMATIONS,
DEFAULT_CALLBACK_GAS_LIMIT,
600,
10_000,
20_000,
VRFCoordinatorV2Plus.FeeConfig({fulfillmentFlatFeeLinkPPM: 200, fulfillmentFlatFeeEthPPM: 100})
);
registerProvingKey();
testConsumer.setCoordinator(v1CoordinatorAddr);
testConsumer_noLink.setCoordinator(v1CoordinatorAddr_noLink);
}

function testDeregister() public {
Expand Down Expand Up @@ -181,6 +212,78 @@ contract VRFCoordinatorV2Plus_Migration is BaseTest {
);
}

function testMigrationNoLink() public {
v1Coordinator_noLink.fundSubscriptionWithEth{value: DEFAULT_NATIVE_FUNDING}(subId_noLink);
v1Coordinator_noLink.addConsumer(subId_noLink, address(testConsumer_noLink));

// subscription exists in V1 coordinator before migration
(
uint96 balance,
uint96 ethBalance,
uint64 reqCount,
address owner,
address[] memory consumers
) = v1Coordinator_noLink.getSubscription(subId_noLink);
assertEq(balance, 0);
assertEq(ethBalance, DEFAULT_NATIVE_FUNDING);
assertEq(owner, address(OWNER));
assertEq(consumers.length, 1);
assertEq(consumers[0], address(testConsumer_noLink));

assertEq(v1Coordinator_noLink.s_totalBalance(), 0);
assertEq(v1Coordinator_noLink.s_totalEthBalance(), DEFAULT_NATIVE_FUNDING);

// Update consumer to point to the new coordinator
vm.expectEmit(
false, // no first indexed field
false, // no second indexed field
false, // no third indexed field
true // check data fields
);
emit MigrationCompleted(v2CoordinatorAddr_noLink, subId_noLink);
v1Coordinator_noLink.migrate(subId_noLink, v2CoordinatorAddr_noLink);

// subscription no longer exists in v1 coordinator after migration
vm.expectRevert(SubscriptionAPI.InvalidSubscription.selector);
v1Coordinator_noLink.getSubscription(subId);
assertEq(v1Coordinator_noLink.s_totalBalance(), 0);
assertEq(v1Coordinator_noLink.s_totalEthBalance(), 0);
assertEq(linkToken.balanceOf(v1CoordinatorAddr_noLink), 0);
assertEq(v1CoordinatorAddr_noLink.balance, 0);

// subscription exists in v2 coordinator
(owner, consumers, balance, ethBalance) = v2Coordinator_noLink.getSubscription(subId_noLink);
assertEq(owner, address(OWNER));
assertEq(consumers.length, 1);
assertEq(consumers[0], address(testConsumer_noLink));
assertEq(balance, 0);
assertEq(ethBalance, DEFAULT_NATIVE_FUNDING);
assertEq(v2Coordinator_noLink.s_totalLinkBalance(), 0);
assertEq(v2Coordinator_noLink.s_totalNativeBalance(), DEFAULT_NATIVE_FUNDING);
assertEq(linkToken.balanceOf(v2CoordinatorAddr_noLink), 0);
assertEq(v2CoordinatorAddr_noLink.balance, DEFAULT_NATIVE_FUNDING);

// calling migrate again on V1 coordinator should fail
vm.expectRevert(SubscriptionAPI.InvalidSubscription.selector);
v1Coordinator_noLink.migrate(subId_noLink, v2CoordinatorAddr_noLink);

// test request still works after migration
testConsumer_noLink.requestRandomWords(
DEFAULT_CALLBACK_GAS_LIMIT,
DEFAULT_REQUEST_CONFIRMATIONS,
DEFAULT_NUM_WORDS,
KEY_HASH,
false
);
assertEq(testConsumer_noLink.s_recentRequestId(), 1);

v2Coordinator_noLink.fulfillRandomWords(testConsumer_noLink.s_recentRequestId());
assertEq(
testConsumer_noLink.getRandomness(testConsumer_noLink.s_recentRequestId(), 0),
v2Coordinator_noLink.generateFakeRandomness(testConsumer_noLink.s_recentRequestId())[0]
);
}

function testMigrateRevertsWhenInvalidCoordinator() external {
address invalidCoordinator = makeAddr("invalidCoordinator");

Expand Down Expand Up @@ -227,6 +330,7 @@ contract VRFCoordinatorV2Plus_Migration is BaseTest {
function registerProvingKey() public {
uint256[2] memory uncompressedKeyParts = this.getProvingKeyParts(UNCOMPRESSED_PUBLIC_KEY);
v1Coordinator.registerProvingKey(OWNER, uncompressedKeyParts);
v1Coordinator_noLink.registerProvingKey(OWNER, uncompressedKeyParts);
}

// note: Call this function via this.getProvingKeyParts to be able to pass memory as calldata and
Expand Down
19 changes: 17 additions & 2 deletions contracts/test/v0.8/foundry/vrf/VRFV2Plus.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ 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 {VRFCoordinatorV2Plus} from "../../../../src/v0.8/dev/vrf/VRFCoordinatorV2Plus.sol";
import {SubscriptionAPI} from "../../../../src/v0.8/dev/vrf/SubscriptionAPI.sol";
import {BlockhashStore} from "../../../../src/v0.8/dev/BlockhashStore.sol";
import {VRFV2PlusConsumerExample} from "../../../../src/v0.8/dev/vrf/testhelpers/VRFV2PlusConsumerExample.sol";
import {VRFV2PlusClient} from "../../../../src/v0.8/dev/vrf/libraries/VRFV2PlusClient.sol";
Expand All @@ -31,6 +32,7 @@ contract VRFV2Plus is BaseTest {

BlockhashStore s_bhs;
ExposedVRFCoordinatorV2Plus s_testCoordinator;
ExposedVRFCoordinatorV2Plus s_testCoordinator_noLink;
VRFV2PlusConsumerExample s_testConsumer;
MockLinkToken s_linkToken;
MockV3Aggregator s_linkEthFeed;
Expand All @@ -57,9 +59,8 @@ contract VRFV2Plus is BaseTest {
s_bhs = new BlockhashStore();

// Deploy coordinator and consumer.
// Note: adding contract deployments to this section will require the VRF proofs be regenerated.
s_testCoordinator = new ExposedVRFCoordinatorV2Plus(address(s_bhs));

// Deploy link token and link/eth feed.
s_linkToken = new MockLinkToken();
s_linkEthFeed = new MockV3Aggregator(18, 500000000000000000); // .5 ETH (good for testing)

Expand All @@ -81,6 +82,8 @@ contract VRFV2Plus is BaseTest {
}
s_testConsumer = VRFV2PlusConsumerExample(consumerCreate2Address);

s_testCoordinator_noLink = new ExposedVRFCoordinatorV2Plus(address(s_bhs));

// Configure the coordinator.
s_testCoordinator.setLINKAndLINKETHFeed(address(s_linkToken), address(s_linkEthFeed));
}
Expand Down Expand Up @@ -142,6 +145,18 @@ contract VRFV2Plus is BaseTest {
s_testCoordinator.fundSubscriptionWithEth{value: 10 ether}(subId);
}

function testCancelSubWithNoLink() public {
uint256 subId = s_testCoordinator_noLink.createSubscription();
s_testCoordinator_noLink.fundSubscriptionWithEth{value: 1000 ether}(subId);

assertEq(LINK_WHALE.balance, 9000 ether);
s_testCoordinator_noLink.cancelSubscription(subId, LINK_WHALE);
assertEq(LINK_WHALE.balance, 10_000 ether);

vm.expectRevert(SubscriptionAPI.InvalidSubscription.selector);
s_testCoordinator_noLink.getSubscription(subId);
}

function testGetActiveSubscriptionIds() public {
uint numSubs = 40;
for (uint i = 0; i < numSubs; i++) {
Expand Down

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ vrf_consumer_v2_upgradeable_example: ../../contracts/solc/v0.8.6/VRFConsumerV2Up
vrf_coordinator_mock: ../../contracts/solc/v0.8.6/VRFCoordinatorMock.abi ../../contracts/solc/v0.8.6/VRFCoordinatorMock.bin 5c495cf8df1f46d8736b9150cdf174cce358cb8352f60f0d5bb9581e23920501
vrf_coordinator_v2: ../../contracts/solc/v0.8.6/VRFCoordinatorV2.abi ../../contracts/solc/v0.8.6/VRFCoordinatorV2.bin 75c87cf1624a401ac6303df9c8e04896aa8a53849e8b0c3d7340a9d089ef6d4b
vrf_coordinator_v2_plus_v2_example: ../../contracts/solc/v0.8.6/VRFCoordinatorV2Plus_V2Example.abi ../../contracts/solc/v0.8.6/VRFCoordinatorV2Plus_V2Example.bin 50d881ecf2551c0e56b84cb932f068b703b38b00c73eb05d4e4b80754ecf6b6d
vrf_coordinator_v2plus: ../../contracts/solc/v0.8.6/VRFCoordinatorV2Plus.abi ../../contracts/solc/v0.8.6/VRFCoordinatorV2Plus.bin 03bcd9234a008f13fb9b0bc8bb28d80b93c294092769e4cebb3c8c8ba5f56950
vrf_coordinator_v2plus: ../../contracts/solc/v0.8.6/VRFCoordinatorV2Plus.abi ../../contracts/solc/v0.8.6/VRFCoordinatorV2Plus.bin bcd16361198eea25b9ca2fb41f019d0ff02f4e7b05289608c37f2e4c9440754d
vrf_external_sub_owner_example: ../../contracts/solc/v0.8.6/VRFExternalSubOwnerExample.abi ../../contracts/solc/v0.8.6/VRFExternalSubOwnerExample.bin 14f888eb313930b50233a6f01ea31eba0206b7f41a41f6311670da8bb8a26963
vrf_load_test_external_sub_owner: ../../contracts/solc/v0.8.6/VRFLoadTestExternalSubOwner.abi ../../contracts/solc/v0.8.6/VRFLoadTestExternalSubOwner.bin 2097faa70265e420036cc8a3efb1f1e0836ad2d7323b295b9a26a125dbbe6c7d
vrf_load_test_ownerless_consumer: ../../contracts/solc/v0.8.6/VRFLoadTestOwnerlessConsumer.abi ../../contracts/solc/v0.8.6/VRFLoadTestOwnerlessConsumer.bin 74f914843cbc70b9c3079c3e1c709382ce415225e8bb40113e7ac018bfcb0f5c
Expand Down

0 comments on commit 748538f

Please sign in to comment.