A $50K smart contract booboo ๐ and how bitwise operations saved us
In the last year Token Page has launched many products, and we're kinda experts now. Our first web3 deployment was MillionDollarTokenPage, and boy did I learn a lot from the ups and downs of it. I'm sharing some of that, now that I know better, with the hope it will help people right at the start of their journey.
So, what happened?
I made a typo in one of the functions. It meant anyone could have minted one of our tokens for free ๐.
The mistake itself was tiny: 10 characters almost cost us $50k.
Web3 can be cruel.
The blockchain is unforgiving
So, I'm sure you all know web3 deployments are unforgiving. Blockchains are immutable ledgers, which means they are purposefully un-editable . But it's hard to adapt to this mindset when you come from web2.
I was previously CTO of a web2 startup. We raised a bunch of money, built an awesome product and grew into an awesome engineering team. One of my the things I was very proud of building there was our CD (continuous deployment) - it meant every engineer had the ability to push code to production many times a day. On a good day we would deploy 100s of updates to our users ๐ฏ.
Of course, we also had CI (continuous integration) which meant almost everything was tested and checked before it was deployed. But really, we all know that actually it's not possibly to catch all bugs before deploying to production.
So when moving to deploying something to the blockchain I knew i had to change my mindset. I tested all our contracts thoroughly before deployment. We ran a beta version on Rinkeby for 3 months with updates pretty much every week.
And still something slipped through.
The most common mistakes in smart contracts
Generally, the biggest mistake people make in smart contracts are things which lead to hacks or stuck funds. Most commonly reported hacks are actually not related to the contract itself but individuals mistakenly giving their credentials to nefarious third parties. I'm going to ignore these for now because as developers we can't fully solve these during the contract writing phase (although we can definitely help - more on this coming soon so sign up for updates).
Our smart contract only took a couple of days to write. We iterated on it at least 50 times, but overall, its still only 420 lines long (seriously ๐).
We had the main worry most smart contract engineers have - how do we make sure everything is safe and nobody can access / transfer anything that doesn't belong to them. So we tested the hell out of this, and we're very sure nobody can.
Side note here: most people simply extend the OpenZeppelin contracts but we needed to rewrite much of it which meant our surface area for hacks was higher than an average NFT contract.
The actual problem
So, despite all the unit-testing, beta-testing and calling everyone we knew to read through the contract, something still slipped through. Here it is:
(if you can spot the error without scrolling down tweet me @krishan711)
It's really as simple as we forgot to add onlyOwner
to the mintTokenGroupAdmin
function ๐คฆโโ๏ธ.
Smart contracts often have "admin" or owner functions that only privileged wallets can execute. We added the same for our contract for giveaways and the like. Annoyingly this leads to bugs that easily to go unnoticed - we have so many tests for minting but we test the admin functions less heavily because we know we wouldn't misuse them and we assume only we can call them.
Well, lesson learnt.
But more interesting is the impact of this and how to recover from it. Let's keep going...
The impact
So basically once someone realised this they could mint as many tokens as they wanted for free. Luckily nobody had and we quickly set about trying to fix it.
But the smart contract is immutable and there's no way to prevent or remove this function. Annoyingly we didn't even have a pause function in the contract or similar (I was still learning back then).
At this point we had two options - 1) declare that everything is basically free now or 2) develop a new contract with a fix and figure out how to migrate the old owners over.
Option 1 isn't an option for us. We rarely give up and we are damn well not doing it when people have trusted us with money.
The main challenge with option 2 is the migration - how can the new contract know which of the old tokens are owned by which owners. this is what leads to our cost estimate. At the peak of ETH and gas costs this year it would have taken up to $50k for us to re-mint the tokens on the new contract and send them to the correct owners.
The solution
We developed a solution that ended up costing us about $2k. It's quite cool so let's go into the details.
The solution we came up with was to have the new contract (v2) have a reference to the old contract (v1). For any tokens that had been minted before v2 was deployed, we tell the v2 contract to read from the v1 contract instead of its own data.
So, how does v2 know which tokens were minted before it was deployed? unfortunately this would require having a hardcoded list on the v2 contract with the v1 minted token ids. We could do it like this:
BUT, filling this with ~1.5k tokens (what had been minted so far) would cost us ~$10k.
$10k is better than $50k, but still way too expensive.
The reason the cost is so high is we are using a list of uint 256 to store token ids. This follows the general ERC-721 spec as developed in OpenZeppelin, but we know in MDTP we only have a max token id of 9999 so actually 14bits would be enough. This is a huge waste.
We realised we don't actually need to store the token ids themselves, we only need to store enough information to know if a specific token id was minted before. So effectively we need 1.5k bools, like this:
This is still quite expensive because in solidity a bool
is stored internally as uint8
. That said, we're definitely moving in the right direction.
How to migrate tokens between contracts
The next breakthrough was realising that we wished bools were stored as bits. then we could simply have an array of 10k bools each "on" or "off" depending in whether the token was minted in v1 or not. This is how we got to the final answer.
We use uint256 fields to store values and use bitwise operations to know if a specific token was minted in v1 or not. So if we had 40 of these ints we could stored the entire mapping of which v1 tokens were minted and which weren't. Let's visualise this:
And here's what the solidity looks like:
I found a very helpful bitwise operations example on github and customised it for our use.
And that's it! Now we just have to set these tokens (through an admin function that is thoroughly tested). This cost a bit of gas but in the end we managed to fix it for ~$1.5k which was much more acceptable to us.
Get in touch
We'd love to hear what you think of our products and are actively looking for interesting use cases. You can find out more at Token Page.
If you have contract troubles, looking to build your own web3 products, or just wanna talk web3 in general, reach out to us, we'd love to help ๐ ย