Skip to content

Commit

Permalink
Fix TOB-7 lack of event generation (#12318)
Browse files Browse the repository at this point in the history
* Emit events for vrf v2plus state changing functions

* Emit FallbackWeiPerUnitLinkUsed event if the fallback is used in _getFeedData

* Add missing tests

* Refactor _getFeedData to return isFeedStale instead of emitting event

* Add enable, disable, and withdrawn events to VRFV2PlusWrapper

* Fix solhint and prettier

* Remove isFeedStale from VRFV2WrapperConsumerBase

* Update Solidity wrappers
  • Loading branch information
leeyikjiun authored Mar 13, 2024
1 parent e8405d9 commit 4b8f36c
Show file tree
Hide file tree
Showing 22 changed files with 2,735 additions and 60 deletions.
2 changes: 2 additions & 0 deletions contracts/src/v0.8/vrf/dev/VRFConsumerBaseV2Plus.sol
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,8 @@ abstract contract VRFConsumerBaseV2Plus is IVRFMigratableConsumerV2Plus, Confirm
*/
function setCoordinator(address _vrfCoordinator) public override onlyOwnerOrCoordinator {
s_vrfCoordinator = IVRFCoordinatorV2Plus(_vrfCoordinator);

emit CoordinatorSet(_vrfCoordinator);
}

modifier onlyOwnerOrCoordinator() {
Expand Down
35 changes: 22 additions & 13 deletions contracts/src/v0.8/vrf/dev/VRFCoordinatorV2_5.sol
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,8 @@ contract VRFCoordinatorV2_5 is VRF, SubscriptionAPI, IVRFCoordinatorV2Plus {
uint8 linkPremiumPercentage
);

event FallbackWeiPerUnitLinkUsed(uint256 requestId, int256 fallbackWeiPerUnitLink);

constructor(address blockhashStore) SubscriptionAPI() {
BLOCKHASH_STORE = BlockhashStoreInterface(blockhashStore);
}
Expand Down Expand Up @@ -502,10 +504,17 @@ contract VRFCoordinatorV2_5 is VRF, SubscriptionAPI, IVRFCoordinatorV2Plus {

bool nativePayment = uint8(rc.extraArgs[rc.extraArgs.length - 1]) == 1;

// We want to charge users exactly for how much gas they use in their callback.
// The gasAfterPaymentCalculation is meant to cover these additional operations where we
// decrement the subscription balance and increment the oracles withdrawable balance.
payment = _calculatePaymentAmount(startGas, gasPrice, nativePayment, onlyPremium);
// stack too deep error
{
// We want to charge users exactly for how much gas they use in their callback.
// The gasAfterPaymentCalculation is meant to cover these additional operations where we
// decrement the subscription balance and increment the oracles withdrawable balance.
bool isFeedStale;
(payment, isFeedStale) = _calculatePaymentAmount(startGas, gasPrice, nativePayment, onlyPremium);
if (isFeedStale) {
emit FallbackWeiPerUnitLinkUsed(output.requestId, s_fallbackWeiPerUnitLink);
}
}

_chargePayment(payment, nativePayment, rc.subId);

Expand Down Expand Up @@ -539,9 +548,9 @@ contract VRFCoordinatorV2_5 is VRF, SubscriptionAPI, IVRFCoordinatorV2Plus {
uint256 weiPerUnitGas,
bool nativePayment,
bool onlyPremium
) internal view returns (uint96) {
) internal view returns (uint96, bool) {
if (nativePayment) {
return _calculatePaymentAmountNative(startGas, weiPerUnitGas, onlyPremium);
return (_calculatePaymentAmountNative(startGas, weiPerUnitGas, onlyPremium), false);
}
return _calculatePaymentAmountLink(startGas, weiPerUnitGas, onlyPremium);
}
Expand Down Expand Up @@ -569,9 +578,8 @@ contract VRFCoordinatorV2_5 is VRF, SubscriptionAPI, IVRFCoordinatorV2Plus {
uint256 startGas,
uint256 weiPerUnitGas,
bool onlyPremium
) internal view returns (uint96) {
int256 weiPerUnitLink;
weiPerUnitLink = _getFeedData();
) internal view returns (uint96, bool) {
(int256 weiPerUnitLink, bool isFeedStale) = _getFeedData();
if (weiPerUnitLink <= 0) {
revert InvalidLinkWeiPrice(weiPerUnitLink);
}
Expand All @@ -594,18 +602,19 @@ contract VRFCoordinatorV2_5 is VRF, SubscriptionAPI, IVRFCoordinatorV2Plus {
if (payment > 1e27) {
revert PaymentTooLarge(); // Payment + fee cannot be more than all of the link in existence.
}
return uint96(payment);
return (uint96(payment), isFeedStale);
}

function _getFeedData() private view returns (int256 weiPerUnitLink) {
function _getFeedData() private view returns (int256 weiPerUnitLink, bool isFeedStale) {
uint32 stalenessSeconds = s_config.stalenessSeconds;
uint256 timestamp;
(, weiPerUnitLink, , timestamp, ) = LINK_NATIVE_FEED.latestRoundData();
// solhint-disable-next-line not-rely-on-time
if (stalenessSeconds > 0 && stalenessSeconds < block.timestamp - timestamp) {
isFeedStale = stalenessSeconds > 0 && stalenessSeconds < block.timestamp - timestamp;
if (isFeedStale) {
weiPerUnitLink = s_fallbackWeiPerUnitLink;
}
return weiPerUnitLink;
return (weiPerUnitLink, isFeedStale);
}

/**
Expand Down
52 changes: 41 additions & 11 deletions contracts/src/v0.8/vrf/dev/VRFV2PlusWrapper.sol
Original file line number Diff line number Diff line change
Expand Up @@ -152,9 +152,11 @@ contract VRFV2PlusWrapper is ConfirmedOwner, TypeAndVersionInterface, VRFConsume
if (address(s_link) != address(0)) {
revert LinkAlreadySet();
}
s_link = LinkTokenInterface(link);

s_link = LinkTokenInterface(link);
s_linkNativeFeed = AggregatorV3Interface(linkNativeFeed);

emit LinkAndLinkNativeFeedSet(link, linkNativeFeed);
}

/**
Expand All @@ -163,6 +165,8 @@ contract VRFV2PlusWrapper is ConfirmedOwner, TypeAndVersionInterface, VRFConsume
*/
function setFulfillmentTxSize(uint32 size) external onlyOwner {
s_fulfillmentTxSizeBytes = size;

emit FulfillmentTxSizeSet(size);
}

/**
Expand Down Expand Up @@ -216,6 +220,18 @@ contract VRFV2PlusWrapper is ConfirmedOwner, TypeAndVersionInterface, VRFConsume
s_fallbackWeiPerUnitLink = _fallbackWeiPerUnitLink;
s_fulfillmentFlatFeeLinkPPM = _fulfillmentFlatFeeLinkPPM;
s_fulfillmentFlatFeeNativePPM = _fulfillmentFlatFeeNativePPM;

emit ConfigSet(
_wrapperGasOverhead,
_coordinatorGasOverhead,
_wrapperPremiumPercentage,
_keyHash,
_maxNumWords,
_stalenessSeconds,
_fallbackWeiPerUnitLink,
_fulfillmentFlatFeeLinkPPM,
_fulfillmentFlatFeeNativePPM
);
}

/**
Expand Down Expand Up @@ -288,7 +304,7 @@ contract VRFV2PlusWrapper is ConfirmedOwner, TypeAndVersionInterface, VRFConsume
function calculateRequestPrice(
uint32 _callbackGasLimit
) external view override onlyConfiguredNotDisabled returns (uint256) {
int256 weiPerUnitLink = _getFeedData();
(int256 weiPerUnitLink, ) = _getFeedData();
return _calculateRequestPrice(_callbackGasLimit, tx.gasprice, weiPerUnitLink);
}

Expand All @@ -311,7 +327,7 @@ contract VRFV2PlusWrapper is ConfirmedOwner, TypeAndVersionInterface, VRFConsume
uint32 _callbackGasLimit,
uint256 _requestGasPriceWei
) external view override onlyConfiguredNotDisabled returns (uint256) {
int256 weiPerUnitLink = _getFeedData();
(int256 weiPerUnitLink, ) = _getFeedData();
return _calculateRequestPrice(_callbackGasLimit, _requestGasPriceWei, weiPerUnitLink);
}

Expand Down Expand Up @@ -385,7 +401,7 @@ contract VRFV2PlusWrapper is ConfirmedOwner, TypeAndVersionInterface, VRFConsume
);
checkPaymentMode(extraArgs, true);
uint32 eip150Overhead = _getEIP150Overhead(callbackGasLimit);
int256 weiPerUnitLink = _getFeedData();
(int256 weiPerUnitLink, bool isFeedStale) = _getFeedData();
uint256 price = _calculateRequestPrice(callbackGasLimit, tx.gasprice, weiPerUnitLink);
// solhint-disable-next-line custom-errors
require(_amount >= price, "fee too low");
Expand All @@ -406,6 +422,10 @@ contract VRFV2PlusWrapper is ConfirmedOwner, TypeAndVersionInterface, VRFConsume
requestGasPrice: uint64(tx.gasprice)
});
lastRequestId = requestId;

if (isFeedStale) {
emit FallbackWeiPerUnitLinkUsed(requestId, s_fallbackWeiPerUnitLink);
}
}

function checkPaymentMode(bytes memory extraArgs, bool isLinkMode) public pure {
Expand Down Expand Up @@ -470,9 +490,12 @@ contract VRFV2PlusWrapper is ConfirmedOwner, TypeAndVersionInterface, VRFConsume
* @param _recipient is the address that should receive the LINK funds.
*/
function withdraw(address _recipient) external onlyOwner {
if (!s_link.transfer(_recipient, s_link.balanceOf(address(this)))) {
uint256 amount = s_link.balanceOf(address(this));
if (!s_link.transfer(_recipient, amount)) {
revert FailedToTransferLink();
}

emit Withdrawn(_recipient, amount);
}

/**
Expand All @@ -481,16 +504,21 @@ contract VRFV2PlusWrapper is ConfirmedOwner, TypeAndVersionInterface, VRFConsume
* @param _recipient is the address that should receive the native funds.
*/
function withdrawNative(address _recipient) external onlyOwner {
(bool success, ) = payable(_recipient).call{value: address(this).balance}("");
uint256 amount = address(this).balance;
(bool success, ) = payable(_recipient).call{value: amount}("");
// solhint-disable-next-line custom-errors
require(success, "failed to withdraw native");

emit NativeWithdrawn(_recipient, amount);
}

/**
* @notice enable this contract so that new requests can be accepted.
*/
function enable() external onlyOwner {
s_disabled = false;

emit Enabled();
}

/**
Expand All @@ -499,6 +527,8 @@ contract VRFV2PlusWrapper is ConfirmedOwner, TypeAndVersionInterface, VRFConsume
*/
function disable() external onlyOwner {
s_disabled = true;

emit Disabled();
}

// solhint-disable-next-line chainlink-solidity/prefix-internal-functions-with-underscore
Expand All @@ -517,18 +547,18 @@ contract VRFV2PlusWrapper is ConfirmedOwner, TypeAndVersionInterface, VRFConsume
}
}

function _getFeedData() private view returns (int256) {
bool staleFallback = s_stalenessSeconds > 0;
function _getFeedData() private view returns (int256 weiPerUnitLink, bool isFeedStale) {
uint32 stalenessSeconds = s_stalenessSeconds;
uint256 timestamp;
int256 weiPerUnitLink;
(, weiPerUnitLink, , timestamp, ) = s_linkNativeFeed.latestRoundData();
// solhint-disable-next-line not-rely-on-time
if (staleFallback && s_stalenessSeconds < block.timestamp - timestamp) {
isFeedStale = stalenessSeconds > 0 && stalenessSeconds < block.timestamp - timestamp;
if (isFeedStale) {
weiPerUnitLink = s_fallbackWeiPerUnitLink;
}
// solhint-disable-next-line custom-errors
require(weiPerUnitLink >= 0, "Invalid LINK wei price");
return weiPerUnitLink;
return (weiPerUnitLink, isFeedStale);
}

/**
Expand Down
4 changes: 4 additions & 0 deletions contracts/src/v0.8/vrf/dev/VRFV2PlusWrapperConsumerBase.sol
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ import {IVRFV2PlusWrapper} from "./interfaces/IVRFV2PlusWrapper.sol";
* @dev fulfillment with the randomness result.
*/
abstract contract VRFV2PlusWrapperConsumerBase {
event LinkTokenSet(address link);

error LINKAlreadySet();
error OnlyVRFWrapperCanFulfill(address have, address want);

Expand Down Expand Up @@ -57,6 +59,8 @@ abstract contract VRFV2PlusWrapperConsumerBase {
}

s_linkToken = LinkTokenInterface(_link);

emit LinkTokenSet(_link);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ pragma solidity ^0.8.0;
/// @notice method required to be implemented by all V2Plus consumers.
/// @dev This interface is designed to be used in VRFConsumerBaseV2Plus.
interface IVRFMigratableConsumerV2Plus {
event CoordinatorSet(address vrfCoordinator);

/// @notice Sets the VRF Coordinator address
/// @notice This method is should only be callable by the coordinator or contract owner
function setCoordinator(address vrfCoordinator) external;
Expand Down
19 changes: 19 additions & 0 deletions contracts/src/v0.8/vrf/dev/interfaces/IVRFV2PlusWrapper.sol
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,25 @@
pragma solidity ^0.8.0;

interface IVRFV2PlusWrapper {
event LinkAndLinkNativeFeedSet(address link, address linkNativeFeed);
event FulfillmentTxSizeSet(uint32 size);
event ConfigSet(
uint32 wrapperGasOverhead,
uint32 coordinatorGasOverhead,
uint8 wrapperPremiumPercentage,
bytes32 keyHash,
uint8 maxNumWords,
uint32 stalenessSeconds,
int256 fallbackWeiPerUnitLink,
uint32 fulfillmentFlatFeeLinkPPM,
uint32 fulfillmentFlatFeeNativePPM
);
event FallbackWeiPerUnitLinkUsed(uint256 requestId, int256 fallbackWeiPerUnitLink);
event Withdrawn(address indexed to, uint256 amount);
event NativeWithdrawn(address indexed to, uint256 amount);
event Enabled();
event Disabled();

/**
* @return the request ID of the most recent VRF V2 request made by this wrapper. This should only
* be relied option within the same transaction that the request was made.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ contract ExposedVRFCoordinatorV2_5 is VRFCoordinatorV2_5 {
uint256 weiPerUnitGas,
bool nativePayment,
bool onlyPremium
) external view returns (uint96) {
) external view returns (uint96, bool) {
return _calculatePaymentAmount(startGas, weiPerUnitGas, nativePayment, onlyPremium);
}
}
22 changes: 22 additions & 0 deletions contracts/test/v0.8/foundry/vrf/VRFV2Plus.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -363,6 +363,7 @@ contract VRFV2Plus is BaseTest {
bytes extraArgs,
bool success
);
event FallbackWeiPerUnitLinkUsed(uint256 requestId, int256 fallbackWeiPerUnitLink);

function testRequestAndFulfillRandomWordsNative() public {
(
Expand Down Expand Up @@ -449,6 +450,27 @@ contract VRFV2Plus is BaseTest {
assertApproxEqAbs(linkBalanceAfter, linkBalanceBefore - 8.3234 * 1e17, 1e15);
}

function testRequestAndFulfillRandomWordsLINK_FallbackWeiPerUnitLinkUsed() public {
(
VRF.Proof memory proof,
VRFCoordinatorV2_5.RequestCommitment memory rc,
,
uint256 requestId
) = setupSubAndRequestRandomnessLINKPayment();

(, , , uint32 stalenessSeconds, , , , , ) = s_testCoordinator.s_config();
int256 fallbackWeiPerUnitLink = s_testCoordinator.s_fallbackWeiPerUnitLink();

// Set the link feed to be stale.
(uint80 roundId, int256 answer, uint256 startedAt, , ) = s_linkNativeFeed.latestRoundData();
uint256 timestamp = block.timestamp - stalenessSeconds - 1;
s_linkNativeFeed.updateRoundData(roundId, answer, timestamp, startedAt);

vm.expectEmit(false, false, false, true, address(s_testCoordinator));
emit FallbackWeiPerUnitLinkUsed(requestId, fallbackWeiPerUnitLink);
s_testCoordinator.fulfillRandomWords(proof, rc, false);
}

function setupSubAndRequestRandomnessLINKPayment()
internal
returns (VRF.Proof memory proof, VRFCoordinatorV2_5.RequestCommitment memory rc, uint256 subId, uint256 requestId)
Expand Down
Loading

0 comments on commit 4b8f36c

Please sign in to comment.