I want to use rewardsAmount variable to calculate my rewards:

uint rewardsAmount = 0;

// Only here i can withdraw rewards

function withdrawRewards() onlyOwner external
    require(rewardsAmount != 0);
    rewardsAmount = 0;

// If the contract doesn't have users funds, then i can destroy it

function kill() onlyOwner external
    require(address(this).balance - rewardsAmount == 0);

function someFunction() public
    // some code
    rewardsAmount += 100;

If someone sends to the contract address some ether, I can never kill the contract, right? So how can I prevent ether from being sent to my contract?


No, you can't prevent Ether being sent to your contract.

Remember that Ether can be forcibly sent to an account

Beware of coding an invariant that strictly checks the balance of a contract.

An attacker can forcibly send ether to any account and this cannot be prevented (not even with a fallback function that does a revert()).

The attacker can do this by creating a contract, funding it with 1 wei, and invoking selfdestruct(victimAddress). No code is invoked in victimAddress, so it cannot be prevented. This is also true for block reward which is sent to the address of the miner, which can be any arbitrary address.

Also, since contract addresses can be precomputed, ether can be sent to an address before the contract is deployed.

See SWC-132

Source: https://consensys.github.io/smart-contract-best-practices/recommendations/#remember-that-ether-can-be-forcibly-sent-to-an-account


You cannot reliably reject funds transferred to a contract address. Be careful not to make an attack vector (disabling your selfdestruct) because someone will selfdestruct(yourContract) to show you the error. Ouch!

A more obscure way is a bit of an Easter egg hunt, but not too far fetched. Given a deployer address and a nonce, the address of the deployed contract is predictable, so it's possible to have a positive balance before the contract is deployed. It's useful for esoteric patterns, mischief and possible sabotage if your contract, again, uses address(this).balance for important logic. Keep in mind it might not be what you expect.

I realize it's a contrived example. The real danger here is using address(this).balance for logic. For the benefit of anyone else who comes across, I would discourage selfdestruct generally. Self Destruct is a Bug.

Hope it helps.

  • Thanks a lot! So i need use "function pause() onlyOwner { pause = true }"? And i don't understand this part "because someone will selfdestruct(yourContract) to show you the error." Where i can read more about this? – ffy Jan 22 '20 at 21:05
  • Yes. It's usually better to freeze all the functions instead of self destruct. You also need a modifier, onlyIfRunning and it should require(!pause, "the contract is stopped."). – Rob Hitchens Jan 23 '20 at 2:00
  • Your contract used selfdestruct to send the money to the owner. Someone else can selfdestruct their contract to force money into your contract and that cannot be repelled - the original question (title), so, no you can't. Not 100% reliable, so ... – Rob Hitchens Jan 23 '20 at 2:03
  • 1
    Now i'm understand)) Thanks again, i learned a lot. – ffy Jan 23 '20 at 6:36

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.