Umbra.sol smart account compatibility

[mozrt2]

Hi there,

While working on the Stealth Safes POC over the weekend, we initially intended to use the deployed Umbra.sol contracts to send stealth payments and emit corresponding events.

We however ended up having to deploy a modified version of the original contract with one specific change to make it compatible with smart contract based accounts.

The current implementation of sendEth() in Umbra.sol uses _receiver.transfer(amount) to transfer a payment to a stealth address. Since EIP-2929, gas is limited to 2300 when calling .transfer(). This causes an out of gas error when trying to send a payment to a Safe account via Umbra.sol.

EIP-2930 solves this with access lists, as shown here by the Safe team. While using access lists would solve the issue we are facing, they are unfortunately not widely supported by wallets (e.g. see here for Metamask).

To build a working POC, we therefore decided to modify the contract and replace _receiver.transfer(amount) with _receiver.call{value: amount}("") which does not face gas constraints.

It seems unlikely that we find a way to use access lists compatible with all major wallets based on our research.

We therefore wanted to open this issue to discuss potential ways forward. We see two, with the first being the preferred path:

• Upgrading the current Umbra.sol contract with a new function, sendEthToSmartContract(), where we insert the .call() method and use a reentrancy guard protection
• Using a separate, modified version of Umbra.sol with these changes for smart accounts

[70nyIT]

If I have to set a preferences over the two, I'd vote on the first option, but as @mozrt2 said, I'd love to hear Umbra team thoughts on this

[mds1]

While using access lists would solve the issue we are facing, they are unfortunately not widely supported by wallets (e.g. MetaMask/metamask-extension#11863).

Huh, I'm surprised metamask still doesn't support access lists

To build a working POC, we therefore decided to modify the contract and replace _receiver.transfer(amount) with _receiver.call{value: amount}("") which does not face gas constraints.

This makes sense. I don't remember why we went with .transfer, but I agree it would have been better to forward all gas. I'm guessing we didn't envision a stealth smart contract recipient when the original contract was written a few years ago, and figured it was safer to just forward less gas.

Either way, as you know we're currently working on ERC-5564 and ERC-6538, and will use those contracts as the foundation for the next version of Umbra. @apbendi let me know if you disagree, but I'd prefer not to release an intermediary contract prior to the ERC that just changes the current version's .transfer to .call. This would introduce a lot of work—the subgraph, umbra-js, and frontend all would have to be updated to support both contracts—for just a few months of usage, and it would delay the ERC going live.

Given that, I think the answer for your situation depends on your timeline, i.e. you can either wait for the ERC to be live and leverage the same contracts we build on top, or start with your forked version and pivot to the ERC version down the road.

If you want to DM me your telegram handles, we'd love to chat more about your use case and what you're working on :)

[mozrt2]

Thanks for the feedback - this makes total sense & great to hear you are aligned with fixing this in a future contract version - I'll DM now so we can further discuss on Telegram :)