api3dao / airnode

Airnode monorepo
https://docs.api3.org/
MIT License
165 stars 72 forks source link

Remove RRP-specific withdrawal implementation from Airnode v1 #1877

Closed bbenligiray closed 1 year ago

bbenligiray commented 1 year ago

We should first research if/how often the RRP withdrawal functionality is used on the public chains. Assuming that it's little to none, I propose to remove the withdrawal functionality from Airnode.

Related to https://github.com/api3dao/airnode/issues/1558

Ashar2shahid commented 1 year ago

User's often use RrpRequesterV0 which makes the contract itself the sponsor. The previous guides/tutorials did not have a withdrawal section and user's funds were locked in the sponsorWallets because the contracts had no withdrawal functions. Since then we added a "Withdrawal Section" in airnode/qrng . Arcadium's sponsorWallet has 0.688 eth locked in their sponsorWallet because the QRNG guides at the time didn't go through withdrawals and the user overestimated how much they should charge for each request. QRNG users also request info about withdrawals (but not frequently).

dcroote commented 1 year ago

QRNG users also request info about withdrawals (but not frequently).

For example here is a PR clarifying the lack of withdrawal functionality early on due to a Discord discussion: https://github.com/api3dao/api3-docs/pull/1107

I use withdrawals in nearly every release test (e.g. #1819, #1839, #1842), and like the functionality's existence so that I don't repeatedly leave testnet funds in various sponsor wallets. That said, having withdrawals external to RRP should also be fine and I envision we can trim a lot from the docs / Airnode repo.

bbenligiray commented 1 year ago

Thanks for your input. After thinking a bit more, I realize that simply removing the withdrawal functionality from Airnode in favor of something like https://github.com/api3dao/airnode/issues/1558#issuecomment-1712127363 will still be breaking, because people will have set sponsors to be contracts and contracts can't sign messages. They can use ERC1271 but that functionality needs to be built in from the start. So it feels like this is too breaking of a change for v0, but we can consider it for v1 (so I edited the issue title).

bbenligiray commented 1 year ago

In fact, the only thing to do about this issue at the moment (in the absence of an Airnode v1) is to update this contract to enable https://github.com/api3dao/airnode/issues/1558 instead. Feel free to re-open the issue if you still think that we should remove withdrawals from v0.