AztecProtocol / aztec-packages

Apache License 2.0
155 stars 157 forks source link

Consider refactoring Token's transfer function with recursion #7142

Open benesjan opened 1 week ago

benesjan commented 1 week ago

Current transfer function is very costly because we try to fetch and work with 32 notes. And given (as Nico would say) that circuits are terrible this results in us having huge gate count because all the gates of note logic are present 32 times in the circuit. Mike described an alternative approach on slack here which leverages recursion. This could allow us to only work with 2 notes by default and recurse if needed (making the base case cheap).

LHerskind commented 1 week ago

We might not need to directly do "recursion" in here, but can just have a smaller amount of notes. Then you can use the multicall structure of the wallets to just perform a few transfers to yourself at the start. Otherwise would still prefer that we don't do it as part of the transfer since the act of making a call is still expensive, so would likely still be cheaper if we would have an "accumulate" instead, where you recurse etc 🤷.

iAmMichaelConnor commented 6 days ago

I think in the best case (having sufficient balance in 1 or 2 notes at the start), the approach I've set out is the most efficient, because then the core transfer function only loops over 2 notes.

Then you can use the multicall structure of the wallets to just perform a few transfers to yourself at the start

the act of making a call is still expensive

Isn't this a contradiction? A multicall results in calls. The approach I've set out doesn't result in any calls if the number of required notes is <= 2.

I also don't think the logic to decide how many times to accumulate should bleed into the wallet (via multicalls); I think it should be part of the token contract, as set out in my proposal.