FundMe

[aviorn36]

Hi Community,
I have doubt regarding a test of withdraw function at timestamp 11:40:01. According to me the test should not have passed at all because after the loop the account connected to the contract is not of deployer but of some random accounts at index 5 retrieved from signer array, so when the withdraw function is called the modifier should have reverted with error (But this is not seen in the test behaviour).
Also, according to my understanding //await fundMeContract.connect(deployer); this should have been added before calling the withdraw function.

    it("withdraw02 - Withdraw ETH when multiple funders", async function () {
      const accounts = await ethers.getSigners();
      for (let index = 1; index < 6; index++) {
        const fundMeNewAccountConnected = await fundMeContract.connect(
          accounts[index]
        );
        await fundMeNewAccountConnected.fund({ value: sendValue });
      }
      const startingFundMeBalance = await fundMeContract.provider.getBalance(
        fundMeContract.address
      );
      const startingDeployerBalance = await fundMeContract.provider.getBalance(
        deployer
      );
     //await fundMeContract.connect(deployer); // according to me this should have been added
     const transactionResponse = await fundMeContract.withdraw();
      const transactionReceipt = await transactionResponse.wait(1);

      const endingFundMeBalance = await fundMeContract.provider.getBalance(
        fundMeContract.address
      );

Can you please let me know if my assumptions is correct or if not then where exactly is understanding lagging?

[AlyKumail]

You are right, but the deployer is already connected to the contract in beforeEach where we getContract : here
beforeEach(async () => { **fundMe = await ethers.getContract("FundMe", deployer)** })
So calling withdraw means the deployer is calling the function.

[aviorn36]

@AlyKumail , I agree that the contract is connected with deployer address in beforeEach, but after that specific test is invoked in it("withdraw02 - ...") there the loop connects the contract with different accounts. To be specific in

for (let index = 1; index < 6; index++) {
        const fundMeNewAccountConnected = await fundMeContract.connect(
          accounts[index]
        );
        await fundMeNewAccountConnected.fund({ value: sendValue });
}

So, this loop scope changes the account that is connected to contract , hence contract now i connected to account/address[5] from which the withdraw function is called and hence it should fail.

[krakxn]

What is the doubt--precisely--here?

[aviorn36]

@krakxn ,
Implementation in contract - " the withdraw function in contract is only allowed to be invoked by the deployer of the contract, so if other accounts calls the withdraw function, the transaction should fail/revert. "

Behaviour observed in test -
It so happens that, we have initially deployer accounts connected in beforeEach from which the contract is deployed locally, now when a test withdraw02 is invoked, we change the accounts connected with the contract in the loop, so when the loop iteration finishes, the contract is NOT connected to the deployer account but with accounts[5] and it is from this account the withdraw function is called and hence it should have failed/revered (due the modifier constraint) but it does-not fails and passes successfully (as shown by @PatrickAlphaC in the tutorial). I hope I am able to state out the ambiguity here.

This is not to point out any issue with the contract function but to point issue with the test implemented in tutorial, the test passes in tutorial, but according to me (wrt the reason/observation stated above) it should have failed.

[alymurtazamemon]

@aviorn36 This instance fundMeContract gets connected with a contract using deployer as an address, and in the loop we are using a different instance fundMeNewAccountConnected and fund the contract with 5 or 6 whatever accounts. After the loop, we are using our old instance back that is still connected with the contract using deployer as an address.

Hope it clears your queries, otherwise, you can tag me anytime, and as soon as I will be available will answer your query.

[alymurtazamemon]

@aviorn36 see connect does not override the previous connect addresses, it simply creates a new instance of contract for that address. So before the loop, we already have an instance that was connected with the contract using the deployer address. In the loop, we created 5 or 6 more, but after the loop, we use our old connected instance for calling withdraw.

You can connect 2 addresses at the same time with a contract, it is not an issue.

[alymurtazamemon]

@aviorn36 I think you are on lesson 7, later you will see that Patrick will connect the contract with 2 addresses, 1 deployer, and 2 player address.

[aviorn36]

@alymurtazamemon , i understood now, key lies in understanding what below 2 lines actually does

fundMeContract = await ethers.getContract("FundMe", deployer);
and
const fundMeNewAccountConnected = await fundMeContract.connect(accounts[index] ); (inside loop)

The first line deploys the contract with the deployer address and the way to interact with this deployed contract is in 2 ways-

1. Using the fundMeContract instance if you trigger any transaction, since this contract instance is already binded to deployer address, it would always use this address.
2. What if we would want to interact with the contract with different address/account? In this case, we would have to tell the blockChain to which smartContract we are really trying to connect and then with what address, so we provide this info using await fundMeContract.connect(accounts[index] ), this line would provide kind of temporary connection/pathway to our same deployed contract with newer address.

I hope I am correct in above understanding. Also, another way, if I would have provided deployer address in connect and used that temporary connection, i would still be able to make the withdraw transaction successfully. Right?
I mean - after loop if I do

 const fundMeNewAccountConnected = await fundMeContract.connect(deployer); // explicitly mentioning deployer in new connection
.
.
.
const transactionResponse = await fundMeNewAccountConnected.withdraw(); // this would still pass?

I know above ways not necessary, I just thought this would/should work too( this would confirm if I have understood correctly).

Also, I believe your line "it simply creates a new instance of contract for that address" is little misleading. For once I thought, this means that there is going to be 5 different instance of contract where we are triggering fund transaction in each. Kind of absurd interpretation, but at a first glance I interpreted in that manner. Writing for future reference for some-one else.

[alymurtazamemon]

@aviorn36 your understood points 1 and 2 correctly, and for the below query, the contract is already connected with the deployer address(we did in beforeEach) but yes this is also the way that you mentioned above.

And yes may be in writing it gives you some confusion but I think now your concepts are clear.

[aviorn36]

@alymurtazamemon thanks once again bro.