Code Audit with OpenZeppelin

Keeping security at a high standard

Published in

Pods

7 min readJan 19, 2023

TLDR: We architected a hybrid system (on-off-chain) where we can find the best of the two worlds: on-chain security with off-chain liquidity. Less than 1% of the TVL is at off-chain risk. This was a good trade-off in our vision. As part of our security layers approach, we finalized two audits with OpenZeppelin along with 2 other audits with CredShields and ABDK. We fixed all critical and high-severity issues.

In the rest of the post, I will explain some architectural decisions and the issues that we decided to not fix for the current version.

You can check all our audits here:

yield-contracts/audits at main · pods-finance/yield-contracts

You can’t perform that action at this time. You signed in with another tab or window. You signed out in another tab or…

github.com

2022: A year to remember why DeFi matters

Celcius, 3AC, BlockFi, and finally FTX. This year, was not hard to find off-chain opportunities that looked amazing. BlockFi was paying more yield on stables than any AAVE/Compound pool. Unfourtanely, when seeking this extra yield, you had the risk of losing it all.

“The transparency and immutability features of the distributed ledger underlying DeFi protocols allow DeFi users to track the entire flow of transactions. Funds in the smart contract are publicly visible and can be audited. Pricing and trading rules are also encoded in the smart contract. As the same algorithm is executed by both parties, the risk of trading errors is eliminated” — New York Fed

With all that off-chain risk, why stETHvv is not fully on-chain?

If we were handling only spot markets, that could work. But the derivatives markets behave differently. In our previous project, the options AMM, we found out how harder is to build derivatives, especially options liquidity on-chain. I would point to three main reasons for that:

Overcollateralization: Option sellers didn’t want to park a lot of capital to be able to sell options. Although we had some approaches like Opyn’s under collateral options, that experience was not as good as the CeFi one.
Pricing AMM: DeFi was easy to follow until Uni V2, but how many people around you really understand Uniswap V3 and how to make money being an LP there? Is it possible to earn money without having to keep moving your liquidity? Imagine for derivatives marketing.
Portfolio Margin: Derivatives traders like to compose their positions by mixing margins of different derivatives types (options, futures, different assets). In DeFi we still don’t have a player that handles this.

In conclusion, we still have more liquidity, and with that, better pricing in off-chain venues. With better pricing, we bring better returns for the end user.

Now that we understood the advantages, how could we protect our strategy and our users against off-chain risk?

The separation between Principal and Yield.

What is on-chain? The Principal
What is off-chain? 50% of the weekly

Principal capital is the part of your capital that will be invested in what we call “Yield Source”. Yield source is the source of yield that will be used to buy weekly the derivatives strategy. The Yield source needs to be economically low risk, at least weekly compounding (1 month fixed-rate won’t work for example), and code-wise low risk, meaning: battled tested, big TVL, good audits, no admin key risks, etc.

In the case of stETHvv, The yield source is the Lido protocol. It earns yield by the proof of stake mechanism and daily generates interest with a rebasing mechanic.

Ok, but even on-chain strategies have risks, right?

Yes, although the CEX scandals stole the scene, we also had many different on-chain hacks. If you check the rekt leaderboard, the top 4 hacks are already bigger than 2 billion dollars in on-chain hacks.

So…What are some architectural decisions that we did in order to reduce those risks?

We don’t have upgradeable contracts. Meaning, our multisig doesn’t have the power to update the rules of the game.
Admin powers (Multisig) don’t have the power to freeze withdraws.
Admin powers (Multisig) don’t have the power to withdraw on behalf of any user
Admins power can only access 50% of the weekly yield generated by the yield source

The OpenZeppelin audit

Now that we covered some general aspects of our security decisions, let’s deep dive into the audit itself. Remember that the audit, together with a bug bounty program is just another resource inside the security spectrum. We have written previously about the security layers in this blog post, and you can also watch a talk that we did in EthCC[4] about it.

In total, we had 4 audits:

The reason why we decided to do a second audit with OZ is that by the end of audit #1, we decided to implement OZ’s 4626 vault implementation instead of our own, and that changed a lot the codebase state. When we first started coding this vault, the 4626 was only a draft, so we didn’t have the chance to use it since the beginning.

If you want to check the full audit report, you can click on the links above. In this section, we will only cover the issues that we acknowledged instead of fixing them.

Medium Severity

M-01: Funds held in `ETHAdapter` can be drained by anyone

Any ETH or stETH that is mistakenly sent to the ETHAdapter can be drained by any user who performs a withdrawal or redemption on the ETHAdapter.

The title of this issue can be scary, but it only means that if someone sent funds directly by mistake to this contract, they will be stolen by the next person that performs a withdrawal. Adding protection against this would delay our launch and would cause an increase in gas consumption for all users that behave correctly. We may implement this in a future version.

M-05: Non-standard ERC-4626 vault functionality

There are multiple locations in the ERC-4626 BaseVault that do not conform to ERC-4626 specifications:

previewWithdraw does not include withdrawal fees
maxDeposit does not return 0 when deposits are disabled
maxMint does not return 0 when withdrawals are disabled
maxWithdraw does not return 0 when withdrawals are disabled

Consider correcting the above issues to meet the ERC-4626 specifications, allowing future vault developers to expect certain protocol behaviors.

The only part that we did not fix was the previewWithdraw function. It is only a view function that returns how many X shares it will use in case you want to withdraw Y assets. Because of rounding issues, including the fees in the projections was not an easy task that would require us a bigger refactor. For now, that function will work, but will not include fees.

M-07: Refunds will be over-credited in a negative yield event

Deposits added to the queue are point-in-time stETH balance amounts. The stETH token rebases to account for yield, and in the event of slashing, may be subject to a negative yield. In the event that a stETH token rebase is negative between the time a user deposits and calls for a refund, the vault will over credit the user by the rebase difference.

Consider handling the deposits in the queue in stETH share amounts to account for rebase changes on refunds.

This issue would also take a deeper refactor where we would need to add a secondary share system for the queue time. After 1 year, Lido never had a negative yield event. Even if this type of event happens, the size of the damage into the share system would be minimum, so that is why we chose to not fix that issue right now.

M-08: Vault can be placed back into vulnerable low supply state

The BaseVault contract inherits an issue common to ERC-4626 implementations known as the donation attack. When shares are minted to users, they are calculated by multiplying the deposited assets by a ratio of the existing shares to current assets. The result of this calculation is then rounded down, following the ERC specifications. The problem arises when the value of totalAssets is manipulated to induce unfavorable rounding for new users depositing into the vault at the benefit of users who already own shares of the vault.

This issue is well-known in the 4626 implementation as you can check here. For now, the Pods team will perform the initial deposit and won’t move it. This is a trust assumption. Another way that we can do in the future, it is a “burning keys cerimony” where we can do the initial deposit using a secondary contract that is not allowed to withdraw after the initial deposit. That way, the Pods team “sacrifice” the initial deposit to remove the trust assumption.

Those are the issues that worth mentioning, the Low and Note severity are more regarding best code practices and documentation.

Meet OpenZeppelin

OpenZeppelin is the premier crypto cybersecurity technology and services company, trusted by the most used DeFi and NFT projects. Founded in 2015 with the mission to protect the open economy, OpenZeppelin safeguards tens of billions of dollars in funds for leading crypto organizations including Coinbase, Ethereum Foundation, Compound, Aave, TheGraph, and many others.

About Pods

Pods’ mission is to establish frictionless structured products for crypto assets.

Pods have built one of the most innovative and safe tools in the Defi space. The company recently did security audits for its Pods Yield product with OpenZeppelin. With the success of its products, security audits, and brand efforts, Pods has set standards for building Defi methods in a fast-growing sector.

The team is excited to offer a platform focused on infrastructure and serving professional clients, investors looking to diversify their portfolio allocation, and have a safe and different strategy to invest in. And, since this is a DeFi strategy, users can also inspect the contracts and verify that the code is doing exactly what it is supposed to do, being genuinely trustless.

Find Out More About Pods

Strategies, vaults, primitives, or tooling. We are building DeFi as we would like to use it.

Find out more at Pods Yield app: https://pods.finance/

Join the Pods community