Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

remove oracle withdraw and allow contract owner to withdraw #11551

Merged
merged 10 commits into from
Jan 2, 2024
Merged

remove oracle withdraw and allow contract owner to withdraw #11551

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
11ff2f5
remove oracle withdraw and allow contract owner to withdraw
jinhoonbang Dec 12, 2023
e456aac
offchain changes
jinhoonbang Dec 13, 2023
b3c1d09
fix integration tests
jinhoonbang Dec 13, 2023
54a523e
fix integration tests
jinhoonbang Dec 13, 2023
94b1d53
Remove amount arg in withdraw and withdraw all the withdrawable amount
kidambisrinivas Dec 18, 2023
6b4b478
Off-chain changes to remove amount arg in withdraw and withdraw all t…
kidambisrinivas Dec 18, 2023
279fec7
address comments
jinhoonbang Dec 21, 2023
3a7a000
Merge branch 'develop' into VRF-797-remove-oracle-withdraw-and-allow-…
jinhoonbang Dec 21, 2023
14f4877
fix lint issue and small refactor in vrf integration tests
jinhoonbang Dec 21, 2023
a319513
address comment
jinhoonbang Dec 22, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 60 additions & 0 deletions contracts/scripts/native_solc_compile_all_vrfv2plus
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
#!/usr/bin/env bash
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this needed? this is already in https://github.com/smartcontractkit/chainlink/blob/develop/contracts/scripts/native_solc_compile_all_vrf

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the naming convention here is defined here . I don't think maintaining a separate foundry profile (e.g. vrfv2plus) is needed


set -e

echo " ┌──────────────────────────────────────────────┐"
echo " │ Compiling VRF contracts... │"
echo " └──────────────────────────────────────────────┘"

SOLC_VERSION="0.8.6"
OPTIMIZE_RUNS=1000000

SCRIPTPATH="$( cd "$(dirname "$0")" >/dev/null 2>&1 ; pwd -P )"
ROOT="$( cd "$(dirname "$0")" >/dev/null 2>&1 ; cd ../../ && pwd -P )"
python3 -m pip install --require-hashes -r "$SCRIPTPATH"/requirements.txt

solc-select install $SOLC_VERSION
solc-select use $SOLC_VERSION
export SOLC_VERSION=$SOLC_VERSION

compileContract () {
local contract
contract=$(basename "$1" ".sol")

solc @openzeppelin/="$ROOT"/contracts/node_modules/@openzeppelin/ --overwrite --optimize --optimize-runs $OPTIMIZE_RUNS --metadata-hash none \
-o "$ROOT"/contracts/solc/v$SOLC_VERSION/"$contract" \
--abi --bin --allow-paths "$ROOT"/contracts/src/v0.8,"$ROOT"/contracts/node_modules\
"$ROOT"/contracts/src/v0.8/"$1"
}

compileContractAltOpts () {
local contract
contract=$(basename "$1" ".sol")

solc @openzeppelin/="$ROOT"/contracts/node_modules/@openzeppelin/ --overwrite --optimize --optimize-runs "$2" --metadata-hash none \
-o "$ROOT"/contracts/solc/v$SOLC_VERSION/"$contract" \
--abi --bin --allow-paths "$ROOT"/contracts/src/v0.8,"$ROOT"/contracts/node_modules\
"$ROOT"/contracts/src/v0.8/"$1"
}

# VRF V2Plus
compileContract vrf/dev/interfaces/IVRFCoordinatorV2PlusInternal.sol
compileContract vrf/dev/testhelpers/VRFV2PlusConsumerExample.sol
compileContractAltOpts vrf/dev/VRFCoordinatorV2_5.sol 50
compileContract vrf/dev/BatchVRFCoordinatorV2Plus.sol
compileContract vrf/dev/VRFV2PlusWrapper.sol
compileContract vrf/dev/testhelpers/VRFConsumerV2PlusUpgradeableExample.sol
compileContract vrf/dev/testhelpers/VRFMaliciousConsumerV2Plus.sol
compileContract vrf/dev/testhelpers/VRFV2PlusExternalSubOwnerExample.sol
compileContract vrf/dev/testhelpers/VRFV2PlusSingleConsumerExample.sol
compileContract vrf/dev/testhelpers/VRFV2PlusWrapperConsumerExample.sol
compileContract vrf/dev/testhelpers/VRFV2PlusRevertingExample.sol
compileContract vrf/dev/testhelpers/VRFConsumerV2PlusUpgradeableExample.sol
compileContract vrf/dev/testhelpers/VRFV2PlusMaliciousMigrator.sol
compileContract vrf/dev/libraries/VRFV2PlusClient.sol
compileContract vrf/dev/testhelpers/VRFCoordinatorV2Plus_V2Example.sol
compileContract vrf/dev/BlockhashStore.sol
compileContract vrf/dev/TrustedBlockhashStore.sol
compileContract vrf/dev/testhelpers/VRFV2PlusLoadTestWithMetrics.sol
compileContractAltOpts vrf/dev/testhelpers/VRFCoordinatorV2PlusUpgradedVersion.sol 5
compileContract vrf/dev/testhelpers/VRFV2PlusWrapperLoadTestConsumer.sol
10 changes: 6 additions & 4 deletions contracts/src/v0.8/vrf/dev/SubscriptionAPI.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,13 +208,14 @@ abstract contract SubscriptionAPI is ConfirmedOwner, IERC677Receiver, IVRFSubscr
* @param recipient where to send the funds
* @param amount amount to withdraw
*/
function withdraw(address recipient, uint96 amount) external nonReentrant onlyOwner {
function withdraw(address recipient) external nonReentrant onlyOwner {
if (address(LINK) == address(0)) {
revert LinkNotSet();
}
if (s_withdrawableTokens < amount) {
if (s_withdrawableTokens == 0) {
revert InsufficientBalance();
}
uint96 amount = s_withdrawableTokens;
s_withdrawableTokens -= amount;
s_totalBalance -= amount;
if (!LINK.transfer(recipient, amount)) {
Expand All @@ -227,11 +228,12 @@ abstract contract SubscriptionAPI is ConfirmedOwner, IERC677Receiver, IVRFSubscr
* @param recipient where to send the funds
* @param amount amount to withdraw
*/
function withdrawNative(address payable recipient, uint96 amount) external nonReentrant onlyOwner {
if (s_withdrawableNative < amount) {
function withdrawNative(address payable recipient) external nonReentrant onlyOwner {
if (s_withdrawableNative == 0) {
revert InsufficientBalance();
}
// Prevent re-entrancy by updating state before transfer.
uint96 amount = s_withdrawableNative;
s_withdrawableNative -= amount;
s_totalNativeBalance -= amount;
(bool sent, ) = recipient.call{value: amount}("");
Expand Down
14 changes: 7 additions & 7 deletions contracts/test/v0.8/foundry/vrf/VRFV2PlusSubscriptionAPI.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,7 @@ contract VRFV2PlusSubscriptionAPITest is BaseTest {
function testWithdrawNoLink() public {
// CASE: no link token set
vm.expectRevert(SubscriptionAPI.LinkNotSet.selector);
s_subscriptionAPI.withdraw(OWNER, 1 ether);
s_subscriptionAPI.withdraw(OWNER);
}

function testWithdrawInsufficientBalance() public {
Expand All @@ -330,7 +330,7 @@ contract VRFV2PlusSubscriptionAPITest is BaseTest {

// call withdraw
vm.expectRevert(SubscriptionAPI.InsufficientBalance.selector);
s_subscriptionAPI.withdraw(OWNER, 1 ether);
s_subscriptionAPI.withdraw(OWNER);
}

function testWithdrawSufficientBalanceLinkSet() public {
Expand All @@ -355,7 +355,7 @@ contract VRFV2PlusSubscriptionAPITest is BaseTest {
// call Withdraw from owner address
uint256 ownerBalance = linkToken.balanceOf(OWNER);
changePrank(OWNER);
s_subscriptionAPI.withdraw(OWNER, 1 ether);
s_subscriptionAPI.withdraw(OWNER);
// assert link balance of owner
assertEq(linkToken.balanceOf(OWNER) - ownerBalance, 1 ether, "owner link balance incorrect");
// assert state of subscription api
Expand All @@ -371,21 +371,21 @@ contract VRFV2PlusSubscriptionAPITest is BaseTest {
// call WithdrawNative
changePrank(OWNER);
vm.expectRevert(SubscriptionAPI.InsufficientBalance.selector);
s_subscriptionAPI.withdrawNative(payable(OWNER), 1 ether);
s_subscriptionAPI.withdrawNative(payable(OWNER));
}

function testWithdrawLinkInvalidOwner() public {
address invalidAddress = makeAddr("invalidAddress");
changePrank(invalidAddress);
vm.expectRevert("Only callable by owner");
s_subscriptionAPI.withdraw(payable(OWNER), 1 ether);
s_subscriptionAPI.withdraw(payable(OWNER));
}

function testWithdrawNativeInvalidOwner() public {
address invalidAddress = makeAddr("invalidAddress");
changePrank(invalidAddress);
vm.expectRevert("Only callable by owner");
s_subscriptionAPI.withdrawNative(payable(OWNER), 1 ether);
s_subscriptionAPI.withdrawNative(payable(OWNER));
}

function testWithdrawNativeSufficientBalance() public {
Expand All @@ -405,7 +405,7 @@ contract VRFV2PlusSubscriptionAPITest is BaseTest {

// call WithdrawNative from owner address
changePrank(OWNER);
s_subscriptionAPI.withdrawNative(payable(OWNER), 1 ether);
s_subscriptionAPI.withdrawNative(payable(OWNER));
// assert native balance
assertEq(address(OWNER).balance, 1 ether, "owner native balance incorrect");
// assert state of subscription api
Expand Down
32 changes: 16 additions & 16 deletions core/gethwrappers/generated/vrf_coordinator_v2_5/vrf_coordinator_v2_5.go
View file
Open in desktop

Large diffs are not rendered by default.

32 changes: 16 additions & 16 deletions core/gethwrappers/generated/vrf_v2plus_upgraded_version/vrf_v2plus_upgraded_version.go
View file
Open in desktop

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions core/gethwrappers/generation/generated-wrapper-dependency-versions-do-not-edit.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ vrf_consumer_v2_plus_upgradeable_example: ../../contracts/solc/v0.8.6/VRFConsume
vrf_consumer_v2_upgradeable_example: ../../contracts/solc/v0.8.6/VRFConsumerV2UpgradeableExample/VRFConsumerV2UpgradeableExample.abi ../../contracts/solc/v0.8.6/VRFConsumerV2UpgradeableExample/VRFConsumerV2UpgradeableExample.bin f1790a9a2f2a04c730593e483459709cb89e897f8a19d7a3ac0cfe6a97265e6e
vrf_coordinator_mock: ../../contracts/solc/v0.8.6/VRFCoordinatorMock/VRFCoordinatorMock.abi ../../contracts/solc/v0.8.6/VRFCoordinatorMock/VRFCoordinatorMock.bin 5c495cf8df1f46d8736b9150cdf174cce358cb8352f60f0d5bb9581e23920501
vrf_coordinator_v2: ../../contracts/solc/v0.8.6/VRFCoordinatorV2/VRFCoordinatorV2.abi ../../contracts/solc/v0.8.6/VRFCoordinatorV2/VRFCoordinatorV2.bin 295f35ce282060317dfd01f45959f5a2b05ba26913e422fbd4fb6bf90b107006
vrf_coordinator_v2_5: ../../contracts/solc/v0.8.6/VRFCoordinatorV2_5/VRFCoordinatorV2_5.abi ../../contracts/solc/v0.8.6/VRFCoordinatorV2_5/VRFCoordinatorV2_5.bin 89c3392fa43260b010970d656ea04357b131390ae6e63a8e42b007fb4ed37dae
vrf_coordinator_v2_5: ../../contracts/solc/v0.8.6/VRFCoordinatorV2_5/VRFCoordinatorV2_5.abi ../../contracts/solc/v0.8.6/VRFCoordinatorV2_5/VRFCoordinatorV2_5.bin bcad64e1b41278998c94867370fd9c4809b1376ccc004955bc7ed33fe716408c
vrf_coordinator_v2_plus_v2_example: ../../contracts/solc/v0.8.6/VRFCoordinatorV2Plus_V2Example/VRFCoordinatorV2Plus_V2Example.abi ../../contracts/solc/v0.8.6/VRFCoordinatorV2Plus_V2Example/VRFCoordinatorV2Plus_V2Example.bin 4a5b86701983b1b65f0a8dfa116b3f6d75f8f706fa274004b57bdf5992e4cec3
vrf_coordinator_v2plus_interface: ../../contracts/solc/v0.8.6/IVRFCoordinatorV2PlusInternal/IVRFCoordinatorV2PlusInternal.abi ../../contracts/solc/v0.8.6/IVRFCoordinatorV2PlusInternal/IVRFCoordinatorV2PlusInternal.bin 834a2ce0e83276372a0e1446593fd89798f4cf6dc95d4be0113e99fadf61558b
vrf_external_sub_owner_example: ../../contracts/solc/v0.8.6/VRFExternalSubOwnerExample/VRFExternalSubOwnerExample.abi ../../contracts/solc/v0.8.6/VRFExternalSubOwnerExample/VRFExternalSubOwnerExample.bin 14f888eb313930b50233a6f01ea31eba0206b7f41a41f6311670da8bb8a26963
Expand All @@ -93,7 +93,7 @@ vrf_v2_consumer_wrapper: ../../contracts/solc/v0.8.6/VRFv2Consumer/VRFv2Consumer
vrf_v2plus_load_test_with_metrics: ../../contracts/solc/v0.8.6/VRFV2PlusLoadTestWithMetrics/VRFV2PlusLoadTestWithMetrics.abi ../../contracts/solc/v0.8.6/VRFV2PlusLoadTestWithMetrics/VRFV2PlusLoadTestWithMetrics.bin 0a89cb7ed9dfb42f91e559b03dc351ccdbe14d281a7ab71c63bd3f47eeed7711
vrf_v2plus_single_consumer: ../../contracts/solc/v0.8.6/VRFV2PlusSingleConsumerExample/VRFV2PlusSingleConsumerExample.abi ../../contracts/solc/v0.8.6/VRFV2PlusSingleConsumerExample/VRFV2PlusSingleConsumerExample.bin 6226d05afa1664033b182bfbdde11d5dfb1d4c8e3eb0bd0448c8bfb76f5b96e4
vrf_v2plus_sub_owner: ../../contracts/solc/v0.8.6/VRFV2PlusExternalSubOwnerExample/VRFV2PlusExternalSubOwnerExample.abi ../../contracts/solc/v0.8.6/VRFV2PlusExternalSubOwnerExample/VRFV2PlusExternalSubOwnerExample.bin 7541f986571b8a5671a256edc27ae9b8df9bcdff45ac3b96e5609bbfcc320e4e
vrf_v2plus_upgraded_version: ../../contracts/solc/v0.8.6/VRFCoordinatorV2PlusUpgradedVersion/VRFCoordinatorV2PlusUpgradedVersion.abi ../../contracts/solc/v0.8.6/VRFCoordinatorV2PlusUpgradedVersion/VRFCoordinatorV2PlusUpgradedVersion.bin ea03be9ed4fc5540ae6ce6a91b9aab345ea443778afb55f82b06ff91b3c2d301
vrf_v2plus_upgraded_version: ../../contracts/solc/v0.8.6/VRFCoordinatorV2PlusUpgradedVersion/VRFCoordinatorV2PlusUpgradedVersion.abi ../../contracts/solc/v0.8.6/VRFCoordinatorV2PlusUpgradedVersion/VRFCoordinatorV2PlusUpgradedVersion.bin 3bcd359eddf2bc861dda86f9bb10b78bac1e0370badd15f67ae17b35c5d228d4
vrfv2_proxy_admin: ../../contracts/solc/v0.8.6/VRFV2ProxyAdmin/VRFV2ProxyAdmin.abi ../../contracts/solc/v0.8.6/VRFV2ProxyAdmin/VRFV2ProxyAdmin.bin 402b1103087ffe1aa598854a8f8b38f8cd3de2e3aaa86369e28017a9157f4980
vrfv2_reverting_example: ../../contracts/solc/v0.8.6/VRFV2RevertingExample/VRFV2RevertingExample.abi ../../contracts/solc/v0.8.6/VRFV2RevertingExample/VRFV2RevertingExample.bin 1ae46f80351d428bd85ba58b9041b2a608a1845300d79a8fed83edf96606de87
vrfv2_transparent_upgradeable_proxy: ../../contracts/solc/v0.8.6/VRFV2TransparentUpgradeableProxy/VRFV2TransparentUpgradeableProxy.abi ../../contracts/solc/v0.8.6/VRFV2TransparentUpgradeableProxy/VRFV2TransparentUpgradeableProxy.bin fe1a8e6852fbd06d91f64315c5cede86d340891f5b5cc981fb5b86563f7eac3f
Expand Down
23 changes: 23 additions & 0 deletions core/gethwrappers/go_generate_vrfv2plus.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
package gethwrappers

//go:generate go run ./generation/generate/wrap.go ../../contracts/solc/v0.8.6/BlockhashStore/BlockhashStore.abi ../../contracts/solc/v0.8.6/BlockhashStore/BlockhashStore.bin BlockhashStore blockhash_store

// VRF V2Plus
//go:generate go run ./generation/generate/wrap.go ../../contracts/solc/v0.8.6/IVRFCoordinatorV2PlusInternal/IVRFCoordinatorV2PlusInternal.abi ../../contracts/solc/v0.8.6/IVRFCoordinatorV2PlusInternal/IVRFCoordinatorV2PlusInternal.bin IVRFCoordinatorV2PlusInternal vrf_coordinator_v2plus_interface
//go:generate go run ./generation/generate/wrap.go ../../contracts/solc/v0.8.6/BatchVRFCoordinatorV2Plus/BatchVRFCoordinatorV2Plus.abi ../../contracts/solc/v0.8.6/BatchVRFCoordinatorV2Plus/BatchVRFCoordinatorV2Plus.bin BatchVRFCoordinatorV2Plus batch_vrf_coordinator_v2plus
//go:generate go run ./generation/generate/wrap.go ../../contracts/solc/v0.8.6/TrustedBlockhashStore/TrustedBlockhashStore.abi ../../contracts/solc/v0.8.6/TrustedBlockhashStore/TrustedBlockhashStore.bin TrustedBlockhashStore trusted_blockhash_store
//go:generate go run ./generation/generate/wrap.go ../../contracts/solc/v0.8.6/VRFV2PlusConsumerExample/VRFV2PlusConsumerExample.abi ../../contracts/solc/v0.8.6/VRFV2PlusConsumerExample/VRFV2PlusConsumerExample.bin VRFV2PlusConsumerExample vrfv2plus_consumer_example
//go:generate go run ./generation/generate/wrap.go ../../contracts/solc/v0.8.6/VRFCoordinatorV2_5/VRFCoordinatorV2_5.abi ../../contracts/solc/v0.8.6/VRFCoordinatorV2_5/VRFCoordinatorV2_5.bin VRFCoordinatorV2_5 vrf_coordinator_v2_5
//go:generate go run ./generation/generate/wrap.go ../../contracts/solc/v0.8.6/VRFV2PlusWrapper/VRFV2PlusWrapper.abi ../../contracts/solc/v0.8.6/VRFV2PlusWrapper/VRFV2PlusWrapper.bin VRFV2PlusWrapper vrfv2plus_wrapper
//go:generate go run ./generation/generate/wrap.go ../../contracts/solc/v0.8.6/VRFV2PlusWrapperConsumerExample/VRFV2PlusWrapperConsumerExample.abi ../../contracts/solc/v0.8.6/VRFV2PlusWrapperConsumerExample/VRFV2PlusWrapperConsumerExample.bin VRFV2PlusWrapperConsumerExample vrfv2plus_wrapper_consumer_example
//go:generate go run ./generation/generate/wrap.go ../../contracts/solc/v0.8.6/VRFMaliciousConsumerV2Plus/VRFMaliciousConsumerV2Plus.abi ../../contracts/solc/v0.8.6/VRFMaliciousConsumerV2Plus/VRFMaliciousConsumerV2Plus.bin VRFMaliciousConsumerV2Plus vrf_malicious_consumer_v2_plus
//go:generate go run ./generation/generate/wrap.go ../../contracts/solc/v0.8.6/VRFV2PlusSingleConsumerExample/VRFV2PlusSingleConsumerExample.abi ../../contracts/solc/v0.8.6/VRFV2PlusSingleConsumerExample/VRFV2PlusSingleConsumerExample.bin VRFV2PlusSingleConsumerExample vrf_v2plus_single_consumer
//go:generate go run ./generation/generate/wrap.go ../../contracts/solc/v0.8.6/VRFV2PlusExternalSubOwnerExample/VRFV2PlusExternalSubOwnerExample.abi ../../contracts/solc/v0.8.6/VRFV2PlusExternalSubOwnerExample/VRFV2PlusExternalSubOwnerExample.bin VRFV2PlusExternalSubOwnerExample vrf_v2plus_sub_owner
//go:generate go run ./generation/generate/wrap.go ../../contracts/solc/v0.8.6/VRFV2PlusRevertingExample/VRFV2PlusRevertingExample.abi ../../contracts/solc/v0.8.6/VRFV2PlusRevertingExample/VRFV2PlusRevertingExample.bin VRFV2PlusRevertingExample vrfv2plus_reverting_example
//go:generate go run ./generation/generate/wrap.go ../../contracts/solc/v0.8.6/VRFConsumerV2PlusUpgradeableExample/VRFConsumerV2PlusUpgradeableExample.abi ../../contracts/solc/v0.8.6/VRFConsumerV2PlusUpgradeableExample/VRFConsumerV2PlusUpgradeableExample.bin VRFConsumerV2PlusUpgradeableExample vrf_consumer_v2_plus_upgradeable_example
//go:generate go run ./generation/generate/wrap.go ../../contracts/solc/v0.8.6/VRFV2PlusClient/VRFV2PlusClient.abi ../../contracts/solc/v0.8.6/VRFV2PlusClient/VRFV2PlusClient.bin VRFV2PlusClient vrfv2plus_client
//go:generate go run ./generation/generate/wrap.go ../../contracts/solc/v0.8.6/VRFCoordinatorV2Plus_V2Example/VRFCoordinatorV2Plus_V2Example.abi ../../contracts/solc/v0.8.6/VRFCoordinatorV2Plus_V2Example/VRFCoordinatorV2Plus_V2Example.bin VRFCoordinatorV2Plus_V2Example vrf_coordinator_v2_plus_v2_example
//go:generate go run ./generation/generate/wrap.go ../../contracts/solc/v0.8.6/VRFV2PlusMaliciousMigrator/VRFV2PlusMaliciousMigrator.abi ../../contracts/solc/v0.8.6/VRFV2PlusMaliciousMigrator/VRFV2PlusMaliciousMigrator.bin VRFV2PlusMaliciousMigrator vrfv2plus_malicious_migrator
//go:generate go run ./generation/generate/wrap.go ../../contracts/solc/v0.8.6/VRFV2PlusLoadTestWithMetrics/VRFV2PlusLoadTestWithMetrics.abi ../../contracts/solc/v0.8.6/VRFV2PlusLoadTestWithMetrics/VRFV2PlusLoadTestWithMetrics.bin VRFV2PlusLoadTestWithMetrics vrf_v2plus_load_test_with_metrics
//go:generate go run ./generation/generate/wrap.go ../../contracts/solc/v0.8.6/VRFCoordinatorV2PlusUpgradedVersion/VRFCoordinatorV2PlusUpgradedVersion.abi ../../contracts/solc/v0.8.6/VRFCoordinatorV2PlusUpgradedVersion/VRFCoordinatorV2PlusUpgradedVersion.bin VRFCoordinatorV2PlusUpgradedVersion vrf_v2plus_upgraded_version
//go:generate go run ./generation/generate/wrap.go ../../contracts/solc/v0.8.6/VRFV2PlusWrapperLoadTestConsumer/VRFV2PlusWrapperLoadTestConsumer.abi ../../contracts/solc/v0.8.6/VRFV2PlusWrapperLoadTestConsumer/VRFV2PlusWrapperLoadTestConsumer.bin VRFV2PlusWrapperLoadTestConsumer vrfv2plus_wrapper_load_test_consumer
8 changes: 4 additions & 4 deletions core/services/vrf/v2/coordinator_v2x_interface.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ type CoordinatorV2_X interface {
GetConfig(opts *bind.CallOpts) (Config, error)
ParseLog(log types.Log) (generated.AbigenLog, error)
OracleWithdraw(opts *bind.TransactOpts, recipient common.Address, amount *big.Int) (*types.Transaction, error)
Withdraw(opts *bind.TransactOpts, recipient common.Address, amount *big.Int) (*types.Transaction, error)
Withdraw(opts *bind.TransactOpts, recipient common.Address) (*types.Transaction, error)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need a second method here for withdrawing native?

Copy link
Contributor Author

@jinhoonbang jinhoonbang Dec 22, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah we can add it for completeness. It seems that Withdraw() isn't used anywhere currently either

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In terms of test coverage, withdrawNative is tested in integration tests which uses a different interface from this one.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this interface is not tested or is just the Withdraw() method not used/tested? And why do integration tests use a different interface?

Copy link
Contributor Author

@jinhoonbang jinhoonbang Dec 28, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The interface is used and tested but seems that a few methods (withdraw() and withdrawNative()) are unused.

The integration tests use different interface but I'm not sure if we have a good reason for it. I think we have room for some refactoring in integration tests to either create a new shared interface for v2 and v2plus or use the existing one above.

LogsWithTopics(keyHash common.Hash) map[common.Hash][][]log.Topic
Version() vrfcommon.Version
RegisterProvingKey(opts *bind.TransactOpts, oracle *common.Address, publicProvingKey [2]*big.Int) (*types.Transaction, error)
Expand Down Expand Up @@ -131,7 +131,7 @@ func (c *coordinatorV2) OracleWithdraw(opts *bind.TransactOpts, recipient common
return c.coordinator.OracleWithdraw(opts, recipient, amount)
}

func (c *coordinatorV2) Withdraw(opts *bind.TransactOpts, recipient common.Address, amount *big.Int) (*types.Transaction, error) {
func (c *coordinatorV2) Withdraw(opts *bind.TransactOpts, recipient common.Address) (*types.Transaction, error) {
return nil, errors.New("withdraw not implemented for v2")
}

Expand Down Expand Up @@ -289,8 +289,8 @@ func (c *coordinatorV2_5) OracleWithdraw(opts *bind.TransactOpts, recipient comm
return nil, errors.New("oracle withdraw not implemented for v2.5")
}

func (c *coordinatorV2_5) Withdraw(opts *bind.TransactOpts, recipient common.Address, amount *big.Int) (*types.Transaction, error) {
return c.coordinator.Withdraw(opts, recipient, amount)
func (c *coordinatorV2_5) Withdraw(opts *bind.TransactOpts, recipient common.Address) (*types.Transaction, error) {
return c.coordinator.Withdraw(opts, recipient)
}

func (c *coordinatorV2_5) LogsWithTopics(keyHash common.Hash) map[common.Hash][][]log.Topic {
Expand Down
4 changes: 1 addition & 3 deletions integration-tests/actions/vrfv2plus/vrfv2plus_steps.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -845,7 +845,6 @@ func ReturnFundsForFulfilledRequests(client blockchain.EVMClient, coordinator co
Msg("Returning LINK for fulfilled requests")
err = coordinator.Withdraw(
common.HexToAddress(defaultWallet),
linkTotalBalance,
)
if err != nil {
return fmt.Errorf("Error withdrawing LINK from coordinator to default wallet, err: %w", err)
Expand All @@ -855,12 +854,11 @@ func ReturnFundsForFulfilledRequests(client blockchain.EVMClient, coordinator co
return fmt.Errorf("Error getting NATIVE total balance, err: %w", err)
}
l.Info().
Str("Native Token amount", linkTotalBalance.String()).
Str("Native Token amount", nativeTotalBalance.String()).
Str("Returning to", defaultWallet).
Msg("Returning Native Token for fulfilled requests")
err = coordinator.WithdrawNative(
common.HexToAddress(defaultWallet),
nativeTotalBalance,
)
if err != nil {
return fmt.Errorf("Error withdrawing NATIVE from coordinator to default wallet, err: %w", err)
Expand Down
4 changes: 2 additions & 2 deletions integration-tests/contracts/contract_vrf_models.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,8 @@ type VRFCoordinatorV2_5 interface {
GetSubscription(ctx context.Context, subID *big.Int) (vrf_coordinator_v2_5.GetSubscription, error)
OwnerCancelSubscription(subID *big.Int) (*types.Transaction, error)
CancelSubscription(subID *big.Int, to common.Address) (*types.Transaction, error)
Withdraw(recipient common.Address, amount *big.Int) error
WithdrawNative(recipient common.Address, amount *big.Int) error
Withdraw(recipient common.Address) error
WithdrawNative(recipient common.Address) error
GetNativeTokenTotalBalance(ctx context.Context) (*big.Int, error)
GetLinkTotalBalance(ctx context.Context) (*big.Int, error)
FindSubscriptionID(subID *big.Int) (*big.Int, error)
Expand Down
Loading
Loading