As part of our ongoing work to improve the usability of Bloom's voting dApp, today we were working on adding a feature for sending email confirmations to users after their vote was mined. Due to a bug introduced during tonight's deploy, we accidentally sent multiple emails to every user who has voted using our app.
To explain the bug, we will provide context for how we sync with the blockchain on the backend of our app. Every unique poll we post on our dapp is its own smart contract. In order to get the latest vote count, we need to ask the blockchain for all Vote
events it has seen since the poll was created. Our vote totals are weighted by the BLT balance of the voter, so in order to get the total BLT associated with each vote we need to sync every vote for a poll as well as the balance of each voter.
When a poll is still open for voting, we do a full sync against the blockchain evey 5 minutes just in case the voter balances change. We save all of this data to our database using a command called an UPSERT. Basically, it lets us either write a new vote or update a vote (to record the most recently synced block) in one call.
When a new vote is cast, our poll syncing tool picks it up and writes it to the database for the first time. The new email confirmation feature added logic to this part of the codebase, and the intended behavior was to only send a confirmation email when the record was created but not on update. The database library we use, Sequelize, returns a Promise<boolean>
type after calling upsert where the boolean would be true if the record was written. The Promise
part means that the operation in question is still executing. The buggy code looked like this:
if (Vote.upsert(...)) { await enqueueJob('successful-vote-email', {voter}) }
when it should have been:
if (await Vote.upsert(...)) { await enqueueJob('successful-vote-email', {voter}) }
In the former case, the conditional is checking whether the return value (the promise) is truthy. The issue here is that we aren't waiting until the UPSERT is done and, due to how Javascript works, this means that the condition is always evaluated as true. The second example properly waits for the work to finish executing and only sends emails for new votes.
We test everything we write in development, but this bug was not caught due to how we run our development blockchains. We use a tool called ganache-cli in development to simulate a blockchain without actually having to do mining. Ganache has other nice features that don't unnecessarily strain your computer. For example, blocks are only mined when it receives new transactions instead of at a relatively constant rate like on a production blockchain.
When we run our tests in development, the sample votes go through and exactly one email is sent out. Further blocks are not mined, so the poll syncer doesn't do any further creates or updates when it runs because nothing changed at all.
We should catch bugs like this before they are released into production and unfortunately this bug slipped through code review as well. We're making the following changes to our development workflow to help reduce the risk of this happening in the future:
--blockTime 1
locally with ganache-cli so that we are a bit more in sync with the behavior of production blockchains.Thank you to everyone who reported the issue, and again we apologize for any inconvenience caused today.