Beyond Smart Contract Best Practices for UX and Interoperability

This post is intended to spark discussion, not immediately undo a well established approach to secure smart contract development. Read and follow this advice with caution.

I spend a fair bit of time helping to maintain ConsenSys’ Smart Contract Best Practices guide. Given the impact the Best Practices have had on the craft of writing smart contracts in the Ethereum ecosystem, it’s a responsibility I take seriously. I believe everyone writing smart contracts on Ethereum, (in any language not just Solidity), should learn and refer back to our best practices frequently.

I also believe that in order to create the blockchain applications that will truly change how humanity collaborates, we must:

(Picasso may not have actually said that, but you can imagine he did)

This is the first of a short series of articles, in which I’ll discuss when, and how to consider breaking some of the prevailing best practices. We’ll see where it goes, but I expect to give this treatment to the following:

  1. Favor pull over push for external calls
  2. Prefer address.send(), and address.transfer() over address.call()
  3. Use block numbers instead of timestamps

Here we go:

Part 1. Revisiting “pull over push for external calls”

A common recommendation (not only in our documentation), is to make ETH transfers with a “Pull”, rather than a “Push”.

What this means, is that when a transaction results in a contract owing a a balance of ETH to a user, rather than sending the ETH right away, the user should collect their balance with a separate withdrawal transaction. This prevents a failed transfer from blocking the normal operation, and makes the rest of the contract unusable for anyone else.

The image below shows the “Pull over Push” examples in the Best Practices, with the bad example on the left, good example on the right.

This is a very safe recommendation, but it also punishes the 99.9% of people who don’t look like this:

pragma solidity ^0.4.0;
contract RuinsThePartyForEveryone{}

A simple change to the Best Practices pattern, to improve the user experience would be:

  1. try to send
  2. if it fails, simply queue up a refund

Here’s how the example shown above would look if we do that:

All we had to do was add an extra if condition, to handle a failed .send() .

Examples in the wild

BlockParty’s journey from push to pull

My friend Makoto got me thinking about this with his post outlining the evolution (or UX devolution?) of the refund mechanism in BlockParty:

BlockParty is an event application that collects a fee from attendees to save them a spot. If you attend you get your money back, if you don’t attend your money is divided evenly among the attendees.

After the event, the owner calls the payout() function. His post describes the decision to make the following change:

WARNING: in the code on the left, if you invite too many friends you’ll run into a different problem with gas limits.

Before the change, the worst that can happen is the highlighted line returns false, but the transaction marches on. After the change, the worse that can happen is some people forget, or never bother to come claim their payouts. Which pattern is less bad is a judgement call you get to make.

How the ENS Registrar decides if a failed send should throw

Another approach can be seen in the ENS Registrar contract, which implements a fairly complex Vickery auction for name space.

For a little bit of a background, any ETH sent to the Registrar to bid on an auction are held in a unique Deed contract. Each Deed has an owner, and a balance. The Registrar contract can decrease the balance by calling this setBalance() function:

function setBalance(uint newValue, bool throwOnFailure)      
onlyRegistrar
onlyActive
{ // Check if it has enough balance to set the value
if (value < newValue) throw;
value = newValue; // Send the difference to the owner
if (!owner.send(this.balance - newValue) && throwOnFailure) throw;
}

Notice the bool throwOnFailure argument? As the name suggests, it’s used to determine how to handle a failed send. If you look at how it’s used in the code, you’ll notice a pattern:

If the send fails, the call will onlythrow if it interferes with an action by the owner of the deed. Take for example the following scenario:

  1. you reveal a new bid,
  2. becoming the new highest bidder,
  3. causing ETH to be sent to the old highest bidder
  4. but the old highest bidder is a contract which fails to receive the payout,
  5. well too bad.

The Registrar never allows an throwing receiver contract to prevent others from taking action. A call to the Registrar only throws on a failed send, when it prevents the receiver themselves from completing an action.

Conclusion

Once you understand the reasoning behind best practices, you can begin to identify situations where those reasons might be outweighed by other considerations, in this case UX. Choose wisely.

Thank you to Goncalo Sa, Mike Goldin, Simon de la Rouviere for providing feedback on this post. Thank you also to Raine Rupert Revere for her talk at Devcon3, which touches on this idea, and to makoto_inoue for his post which made me write this post.