smartcontractkit / chainlink

node of the decentralized oracle network, bridging on and off-chain computation
https://chain.link
Other
6.95k stars 1.69k forks source link

[DEVEL] VRF AddConsumer is failing #7982

Open reddigszymon opened 1 year ago

reddigszymon commented 1 year ago

Description I am trying to add a consumer to my VRF Chainlink Subscription using my smart contract, but every time i get an error stating that it cannot estimate gas.

Your Environment Hardhat + Windows + Solidity

Basic Information The error once i try to call COORDINATOR.addConsumer() function is as follows:

Error: ERROR processing D:\projects\lottery\deploy\01-deploy-lotteryOneWinner.js: Error: cannot estimate gas; transaction may fail or may require manual gas limit [ See: https://links.ethers.org/v5-errors-UNPREDICTABLE_GAS_LIMIT ] (reason="execution reverted", method="estimateGas", transaction={"from":"0xAc5BB2Ee24FC60fE729BAF80FEEd8025c1D298Ef","data":"0x61014060405260

Steps to Reproduce

You can check out my contract here (vrfCoordinator is hardcoded for the polygon mumbai network):

https://mumbai.polygonscan.com/address/0xa9632B5b79769e1E5Aea0A891804055E9848E2AB#writeContract

The addConsumer function is meant to add the consumer, but it does not work.

0xheartcode commented 1 year ago

Same issue, I have tried using the contract provided here: https://docs.chain.link/vrf/v2/subscription/examples/programmatic-subscription/

The following call works on deployment:

    constructor() VRFConsumerBaseV2(vrfCoordinator) {
        COORDINATOR = VRFCoordinatorV2Interface(vrfCoordinator);
        LINKTOKEN = LinkTokenInterface(link_token_contract);
        s_owner = msg.sender;
        //Create a new subscription when you deploy the contract.
        createNewSubscription();
    }

    function createNewSubscription() private onlyOwner {
        s_subscriptionId = COORDINATOR.createSubscription();
        COORDINATOR.addConsumer(s_subscriptionId, address(this));
    }

As seen above the contract deploys and creates a new subscription and adds a consumer. I originally simply wanted to add a consumer to an existing subscription so I modified the createNewSubscription function.

    // Create a new subscription when the contract is initially deployed.
    function createNewSubscription() private onlyOwner {
        s_subscriptionId = COORDINATOR.createSubscription();
        COORDINATOR.addConsumer(1234, address(this));
    }

This does not work. Of course I already have a subscription(1234) that works and can add manually or with etherscan but for some reason, calling from another contract does not work.

itgav commented 1 year ago

@0xheartcode I haven't delved too into the weeds but just did some testing. The Coordinator seems to be tied to the contract that creates the subscription. I made a separate "addNewConsumer()" function to test this. If two contracts, "A" and "B", are deployed and create subscriptions. Contract "A" can add contract "B" to "A's" subscription, but contract "A" can't add themselves to contract "B's" subscription.

Breaking the above results in the gas estimation error.

0xheartcode commented 1 year ago

@itgav I have found the issue. Looking at VRFCoordinatorV2.sol The function addConsumer has a modifier called onlySubOwner here. Which is not explicitly outlined in the docs.

function addConsumer(uint64 subId, address consumer) external override onlySubOwner(subId) nonReentrant {
    // Already maxed, cannot add any more consumers.
    if (s_subscriptionConfigs[subId].consumers.length == MAX_CONSUMERS) {
      revert TooManyConsumers();
    }
    if (s_consumers[consumer][subId] != 0) {
      // Idempotence - do nothing if already added.
      // Ensures uniqueness in s_subscriptions[subId].consumers.
      return;
    }
    // Initialize the nonce to 1, indicating the consumer is allocated.
    s_consumers[consumer][subId] = 1;
    s_subscriptionConfigs[subId].consumers.push(consumer);

    emit SubscriptionConsumerAdded(subId, consumer);
  }
modifier onlySubOwner(uint64 subId) {
    address owner = s_subscriptionConfigs[subId].owner;
    if (owner == address(0)) {
      revert InvalidSubscription();
    }
    if (msg.sender != owner) {
      revert MustBeSubOwner(owner);
    }
    _;
  }

This means it is not possible to add a consumer if you're not the owner of the subscription.

I was trying to add the subscription via the deployed smart contract with delegate call, but it only accepts direct interactions from the subscription owner (which was my EOA on Metamask). It seems to be a safety feature rather than a bug, the way to go would be to use topUpSubscription to add the LINK token to the subscription programmatically or use the direct funding method.

Including an acceptSubscriptionOwnerTransfer in the deployed contract would be helpful for troubleshooting with an EOA in case of issues.

lancelot-c commented 1 year ago

I have investigated, and indeed, it is not possible for a contract to add itself as a consumer of a subscription that was not created by itself.

The reason is the line 857 of VRFCoordinatorV2.sol:

if (msg.sender != owner) {
   revert MustBeSubOwner(owner);
}

Whenever your contract calls addConsumer, it will go through the onlySubOwner modifier which will check that msg.sender (your contract address) is equal to owner (the address which created the subscription), and revert if the check fails.

So if you have created your subscription manually on ChainLink website using Metamask and then deploy a contract passing the subscription ID to subscribe to it, it will always fail because msg.sender will be equal to your newly deployed contract address and not your Metamask address.

It's quite annoying because it means we don't have a full programmatic way for subscribing to existing subscriptions because in all cases we will always have to send a manual addConsumer transaction outside of our contract.

Would appreciate a fix for that ChainLink Team 🙏

0xheartcode commented 1 year ago

Hi @lancelot-c:

It seems to be a safety feature rather than a bug, the way to go would be to use topUpSubscription to add the LINK token to the subscription programmatically or use the direct funding method.

Including an acceptSubscriptionOwnerTransfer in the deployed contract would be helpful for troubleshooting with an EOA in case of issues.

lancelot-c commented 1 year ago

It seems to be a safety feature rather than a bug, the way to go would be to use topUpSubscription to add the LINK token to the subscription programmatically or use the direct funding method.

Indeed it's probably a safety feature but it would be super convenient for development and debugging purposes to have a way to easily subscribe to an existing subscription without having to go through all the mess of topUp with LINK at each deployment or manually transferring ownership.

I think many developers using ChainLink VRF have a single subscription on Sepolia and want to use the same for all contracts they deploy.

Including an acceptSubscriptionOwnerTransfer in the deployed contract would be helpful for troubleshooting with an EOA in case of issues.

How does it help ? Yes we can transfer the ownership of the subscription to our contract but as I said, it's cumbersome. When you deploy a new contract on Sepolia 10 times a day you don't want to transfer the subscription manually 10 times, it's just not efficient. You want to have an easy method for using the existing subscription straight away.

0xheartcode commented 1 year ago

I agree with you on the point above, I just guessed it was a feature. It did take me a long time as a beginner to understand what was going on, since it was not detailed explicitly.

Regarding how it helps, yes it is messy but in theory, you could script it with some js. For testing purposes it is horrible, agreed.

iminparallel commented 1 week ago

Same issue, I have tried using the contract provided here: https://docs.chain.link/vrf/v2/subscription/examples/programmatic-subscription/

The following call works on deployment:

    constructor() VRFConsumerBaseV2(vrfCoordinator) {
        COORDINATOR = VRFCoordinatorV2Interface(vrfCoordinator);
        LINKTOKEN = LinkTokenInterface(link_token_contract);
        s_owner = msg.sender;
        //Create a new subscription when you deploy the contract.
        createNewSubscription();
    }

    function createNewSubscription() private onlyOwner {
        s_subscriptionId = COORDINATOR.createSubscription();
        COORDINATOR.addConsumer(s_subscriptionId, address(this));
    }

As seen above the contract deploys and creates a new subscription and adds a consumer. I originally simply wanted to add a consumer to an existing subscription so I modified the createNewSubscription function.

    // Create a new subscription when the contract is initially deployed.
    function createNewSubscription() private onlyOwner {
        s_subscriptionId = COORDINATOR.createSubscription();
        COORDINATOR.addConsumer(1234, address(this));
    }

This does not work. Of course I already have a subscription(1234) that works and can add manually or with etherscan but for some reason, calling from another contract does not work.

This was a life saver, I was stuck on this all day https://github.com/Cyfrin/foundry-full-course-cu/discussions/1742