Pickle in a Pickle: A Post-Mortem
Last week, Pickle Finance underwent a migration of PickleJars due to a procedural error. During this time, we wrote an article describing the situation but lacked the bandwidth to flesh out the details.
Ultimately, there was no loss of funds, and this was not caused by any bugs within the smart contracts. But the community deserves to know more about what happened and the reasons behind it.
This article is an account of the events that transpired, including the technical causes, the fund rescue operation, and a subsequent mitigation strategy to prevent a repeat of similar events in the future.
To start, this next section provides technical context for the cause of the incident. Feel free to skip this section if you prefer.
To understand what went wrong, we first need to understand how a regular
withdraw() call to the PickleJar is executed. The following is part of the PickleJar source code:
Only the first and last lines are relevant here, the rest has been omitted for clarity.
In the first line, we make a calculation for the variable
r. This is the amount of the underlying asset that we eventually want to transfer to the user, as reflected by the final line of the function.
However, the calculation for
r depends on calling
balance() of the PickleJar contract, which looks like this:
This calculation actually depends on two components: (1) the balance in the PickleJar contract, plus (2) the balance in the Strategy contract.
The former part of this calculation is clearly shown, but the latter is done through calling the Controller. You can see that the Controller’s
balanceOf() function simply forwards the call to the relevant Strategy:
The overall “balance” of a PickleJar is actually located across two separate contracts: the PickleJar contract and the Strategy contract.
If you skipped the technical section above, the only thing you need to understand is that the overall “balance” of a PickleJar is actually located across two separate contracts: the PickleJar contract and the Strategy contract.
The PickleJar always stays the same, but it delegates most of its functionality to the Controller which handles what Strategy to use. Therefore, changing the Controller also potentially changes what Strategy is being used.
The Great Migration
On Sep 28, 2020, PIP-8 was passed by the community. Part of this proposal involved diverting PickleJar withdrawal fees to the community-governed Treasury and as a result, the existing Strategy and Controller contracts needed to be updated.
Since funds are stored in both the PickleJar contract (which doesn’t need to change) and the Strategy contract (that we want to dispose of), we needed to execute a careful sequence of steps in order to replace both the Strategy and Controller contracts:
- Deploy new Strategy and new Controller contracts;
- Approve and set new Strategy contracts with the new Controller;
withdrawAll()on the old Strategy contract to migrate its funds to the PickleJar contract;
- Since all funds are now within the PickleJar contract, we can safely call
setControllerto migrate to the new Controller contract.
Here is a diagram to help visualize this process:
Where Things Went Wrong
At block 10958758, a series of transactions were called via the Timelock to initiate
setController() on each of the PickleJars. This was the fatal mistake because
withdrawAll() (read Step 3 above) was not yet called, and it resulted in the bulk of underlying assets staying in the old Strategies.
Recall from above that the total balance of a PickleJar is calculated by adding up funds stored in both the PickleJar and Strategy contracts.
As Jar operation continued, funds would be deposited and withdrawn with respect to a (very small) balance that missed all of the funds stored in the old Strategy (since the new Controller pointed to the new Strategies). This meant the system calculated very small payments upon withdrawal and very large pToken issuances on deposit.
The reason is because your withdrawal amount is calculated as a percentage of the total pool. But the total pool size was off, so the amount you ended up getting was much smaller than it should have been.
We also observed evidence of users that tried to exploit the system by manually depositing into the PickleJars through Etherscan to obtain a large amount of pTokens, which resulted in them getting a larger share of PICKLE farming rewards. This continued to happen for many hours after we disabled the frontend and gave several warnings not to do this.
The reason why this happened was due to a miscommunication of intent within the team.
The members that queued the transaction had neglected to warn that the transaction was only to be executed after a
withdrawAll() was called. While the members that executed the transaction had assumed that this was not necessary based on an alternate understanding of the migration.
It is conceivable that the migration could have happened with another sequence of steps. One in which the system updates are done in a piecemeal fashion (i.e. Controller first, Strategy second):
- First upload a new Controller pointing towards the same old Strategy;
setControllerto the PickleJar to point it towards the new Controller;
- Upload and approve the new Strategy;
withdrawAll()on the old Strategy;
setStrategy()on the new Controller to point to the new Strategy.
This misunderstanding of the agreed-upon sequence of steps is what caused the incident.
The Pickle Response
After a user reported a discrepancy with the withdrawal function, the team decided to disable deposit and withdrawal functions on the frontend at 18:17 UTC of Sep 29, 2020. We subsequently also announced (on Twitter) that we would take a snapshot of the system state at block 10959175.
The team then created a snapshot of user balances by using calldata to capture the state of the system. This method captured all PickleJar interaction via the frontend and even Etherscan.
Banteg, a developer for Yearn Finance, recommended capturing Transfer events rather than using calldata. The reason for this is because calldata could potentially miss highly technical users who were depositing and withdrawing via custom-written proxy smart contracts.
The snapshot script was tweaked to use Transfer events and consequently one additional user was added to the resulting snapshot. We thank Banteg for his contribution and review of our script.
Here is the final script we used to generate the snapshot.
The Great Rescue
The gravity of the situation quickly dawned on us, especially because more than $100 million USD was being managed by the PickleJars at the time. From the moment we realized this, we worked non-stop for 24 hours to rescue the funds and create a redistribution strategy that would work for our PickleJar users.
Due to the urgency of the situation, a decision was made by the team to set the dev multi-sig as the new Timelock on the Strategies to bypass the 12 hour Timelock. This change in itself would have to bypass the Timelock, but it would eventually allow the team to execute arbitrary functions on the Strategies.
To ensure that the migration was going to be successful, a series of tests were written and tested against a fork of the Ethereum Mainnet. This assured us that our code would work against real-world state. Once we were certain we would succeed, we deployed the contracts with custom rescue logic (here and here).
When the Timelock passed, the team executed the necessary transactions to rescue approximately $130 million USD worth of LP and sCRV tokens. These were then deposited into the new PickleJars, producing pTokens. And these pTokens were then sent to Solidity contracts for users to claim.
You can now claim your new pTokens by following this blog post we wrote several days ago. These new pTokens entitle you to your stake in the new PickleJars (with the new PIP-8 fee structure).
Since farms on the Pickle protocol are hardcoded to accept a specific pToken (as all Masterchef-based farms are), we had to disable the old farms and replace them with new ones that would accept the new pTokens.
This required transactions to be sent to the Masterchef contract: the sole minter of PICKLE tokens. And since there is a 24 hour Timelock that the Masterchef contract sits behind, this took a minimum of 24 hours to do.
Thankfully, these farms are open again now and are operating normally.
During the time that PickleJar operation was shut down, the funds continued to earn interest from their strategies. All of these gains were calculated via this script.
These funds totalled approximately $232k USD and after we completed the rescue operation, it was sent to the community controlled Treasury in two transactions (here and here). We encourage the community to visit the forum and discuss the logistics of how to use or redistribute these funds.
Going forward, we have decided to establish a system to prevent such a miscommunication from ever happening again. These changes will not only make the dev team’s actions more reliable, but it will also make the protocol operate more transparently with the community at large.
Timelock Transaction Discord Channel
Since crucial functions to the system are almost always behind a Timelock, it makes sense to implement an accountability system at the moment that a Timelock transaction is queued.
As such, a Discord channel will be created specifically for the dev team to announce the queueing of every single Timelock transaction. For each of these, it will be required to state (for everyone to see):
- The context of why the transaction is being executed;
- If there are any pre-requisites that must be done before the transaction is to be executed, and finally;
- The specific personnel expected to execute the transaction.
For the final point, it’s important to also “@” tag them so they are notified of their responsibilities. If there is ever any confusion as to what, when, why, or how a Timelock transaction is to be executed, the tagged person can clarify with the rest of the team.
The benefit of having such a channel in public is that we become more transparent, and members of the community can be more involved in making sure we are executing transactions that make sense.
We are confident that having this system in place will not only prevent a similar incident from happening again, but will actually improve operations and community confidence in the dev team.
Hopefully this (very long) article clarifies the situation a bit and brings a bit more understanding of the situation to the Pickle community.
As always, thank you so much for believing in us. The best is yet to come.
Join our Discord http://discord.gg/gR85hmC
Contributors to Pickle have made reasonable efforts at ensuring the integrity of the protocol including tests. Pickle is completely valueless and has 0 financial value. Anyone who chooses to engage with these contracts, including the Pickle token contract and the staking contracts, are doing so at their own risk. You should perform your own due diligence.