When testing my reentrance, I'm not getting the appropriate revert message.
The function with guard;
function sellShares(uint256 amount) public nonReentrant {
//...
(bool sent, ) = _msgSender().call{value: returnedAmount}("");
require(sent, "Failed to send Ether");
//...
}
when reentering the contract, the call is reverted, but I'm getting the Failed to send Ether message
The thing is that even if I get this message and validate that the reentrance worked. I'm unable to make the test pass.
await expect(await reentry.sellShares())
.to.be.rejectedWith('Failed to send Ether');
The logs from the test
Testing Reentry
Should revert on reentry:
Error: VM Exception while processing transaction: reverted with reason string 'Failed to send Ether'
at . (0x59b670e9fa9d0a427751af201d676719a970857b)
at . (0x322813fd9a801c5507c9de605d63cea4f2ce6c44)
at processTicksAndRejections (node:internal/process/task_queues:96:5)
at HardhatNode._mineBlockWithPendingTxs (node_modules/hardhat/src/internal/hardhat-network/provider/node.ts:1866:23)
at HardhatNode.mineBlock (node_modules/hardhat/src/internal/hardhat-network/provider/node.ts:524:16)
at EthModule._sendTransactionAndReturnHash (node_modules/hardhat/src/internal/hardhat-network/provider/modules/eth.ts:1482:18)
at HardhatNetworkProvider.request (node_modules/hardhat/src/internal/hardhat-network/provider/provider.ts:124:18)
at HardhatEthersSigner.sendTransaction (node_modules/@nomicfoundation/hardhat-ethers/src/signers.ts:125:18)
at send (node_modules/ethers/src.ts/contract/contract.ts:313:20)
I'm not sure if the reentrancy guard should return a specific message but even if i expect this message the test don't pass.
The inner await causes the transaction to get executed and throw an exception.
Subsequently, the expect function itself doesn't even get executed.
So you should get rid of that await and let the expect function deal with it.
Also, I'm pretty sure that you should replace rejectedWith with revertedWith.
Yes i have figured this out but i'm not able to get a reentrancy specific error. Not sure if i should. I'm using the latest OZ contract package.
So why have you posted await expect(await reentry.sellShares()) if this is NOT what you're doing?
This was indeed wrong, but i'm trying to get the revert error from the reentrance. This on is from the call.
And what do you get after removing the inner await?
BTW, with _msgSender().call{value: returnedAmount}(""), you are calling the contract's receive function and not the sellShares function, so in either case, you are not testing reentrancy here.
The test succeed but only with the revert string 'Failed to send Ether'.
I was expecting a string more specific to reentrance
When the contract receive ether. it call for sellShares again.
I have also tried with a fallback but it's the same behaviour.
You've explicitly stated that string in the require statement, why would you expect anything else???
Well then you should add that as part of your question, since it is very much relevant.
Because when of the modifier on that second call. Is it not getting triggered before it get's to the second call?
But you've used call, which doesn't revert, but rather - returns a success status.
You even obtain that return-value explicitly in your code (using bool sent), so you are probably aware of the fact that call doesn't revert:
(bool sent, ) = _msgSender().call{value: returnedAmount}("");
And since this line doesn't revert, it is the next line which reverts:
require(sent, "Failed to send Ether");
And this line reverts with the error-message which you ultimately receive.
So if i remove the require statement i should get that message?
I see. I will experiment with this. thank you!
NO, the sellShares function will just complete successfully!
If you change call to transfer, then the function will revert at the nonReentrant modifier, and you'll then receive the error-message specified in that modifier.
1 Like