refactor: Include logger and sendPayment in useWallets

[ekzyis]

Description

This is a prerequisite for #1642.

Since for #1642, useWallets needs to return wallets with the functionality to pay, I updated the code so we include the payment interface in useWallets.

Since the payment interface requires the wallet logger, I call the logger hooks for each wallet (useWalletLogger) in a loop even though that is against the rules of hooks. To prevent calling hooks in a different order between renders, I am sorting wallets by name before I call the hooks and independent of their priority or if they are even configured.

Based on #1638, #1640, #1641

Additional Context

tbd

Checklist

Are your changes backwards compatible? Please answer below:

yes, useWallet continues to work as before

On a scale of 1-10 how well and how have you QA'd this change and any features it might affect? Please answer below:

tbd

For frontend changes: Tested on mobile, light and dark mode? Please answer below:

n/a

Did you introduce any new environment variables? If so, call them out explicitly here:

no

[huumn]

Usually, this can be avoided by refactoring non-hook stuff out of the hook.

My initial refactor of this code had to do this a lot.

---

It's also going to be tempting to start making useWallets a monolith again, but that's part of what made the code so buggy. Meaning, it's going to be convenient to push all code into useWallets for anything that uses useWallets but it's going to be inconvenient to maintain.

[huumn]

Another thing that might help is figuring out a new pattern for dealing with a hook that returns an array.

currently:

function MyComponent () => {
const wallets = useWallets()
return wallets.map(...)
}
// or
function useSomethingWithAWallet () => {
const wallets = useWallets()
const [state, setState] = useState()

const cb = useCallback(..., [wallets, state])

return cb
}

Our desire seems to be to want to "decorate" a dynamically chosen wallet from useWallets, but we usually do that in a hook for a single wallet, e.g. useWalletLogger.

That's not a super complete picture, but if we enumerate all the problems we're trying to solve, we should be able to come up with an acceptable pattern to deal with this.

[ekzyis]

These are my high-level thoughts:

---

Current problem

useWallets returns an array of wallet configurations which cannot pay.

For sender fallbacks, we need an array of wallets that can pay.

waitForWalletPayment requires a wallet that can pay.

We cannot add the payment interface in waitForWalletPayment to a wallet because this means the wallet needs to be set when we call useWalletPayment else we would call a hook inside a function call and not in the top-level of a React component. And we cannot set the wallet when we call useWalletPayment because we want to use multiple wallets, not just one.

wallet.sendPayment requires the logger which requires to call the useWalletLogger hook.

---

First idea: path of least resistance

• add payment interface in useWallets and thus call useWalletLogger in there, too

Problem: useWallets tends to become a buggy, hard-to-maintain monolith again

Status: DISMISSED

---

Second idea: move code around + better naming

• rename following stuff:
  • WalletsProvider -> WalletConfigsProvider
  • useWallets -> useWalletConfigs
  • useWallet -> useWalletConfig

Since useWallets doesn't return a wallet that can pay but (decrypted) configurations of wallets (which lead me to believe we should add the payment interface there), I think that rename would be a good decision.

• create new useWallets(configs) hook that requires the array of wallet configs and then adds the payment interface to them. We then loop over this for sender fallbacks and pass each wallet to waitForWalletPayment.

• create new useWallet that calls the new useWallets and filters the wallet we want out (just like it does currently)

Problem: ???

Status: OPEN

---

Third idea: make useWalletPayment aware of wallet array

• keep useWallets as-is (prior to this PR) and replace useWalletPayment with something that wraps useWallets and returns wallets with the payment interface like I did in this PR (= useWalletPayments). We then loop over this and can call wallet.sendPayment directly (which already includes the logging code).

Problem: ??? (except that useWallets is still a bad name imo)

Status: OPEN

---

Fourth idea: decouple loggers from wallets

• replace useWalletLogger with useWalletLoggers which returns an array of wallet loggers. I think useWalletLogger doesn't need to know much about a wallet, we could run the little code that we need from useWallets in useWalletLoggers, too.

• merge the arrays returned by useWallets and useWalletLoggers in a new hook to return an array of wallets with the payment interface

Problem: see below

Status: OPEN

---

Fifth idea: a mix of all ideas

Not much to say here. Just a filler option to acknowledge that there might be something good in each idea and to make the picture below a better fit.

Status: OPEN

---

In all ideas except the fourth one, we need to call useWalletLogger in a for loop that doesn't break the rules of hooks (= consistent order between renders). However, I don't really like the fourth idea. I prefer mapping and filtering over merging arrays. Additionally, I think only using walletDefs in one place (useWallets) is better than also using it in useWalletLoggers to pick out of it what we need without calling useWallets.

[huumn]

Second idea: move code around + better naming

I think this one is the winner. It sounds like we just need to start composing hooks.

[ekzyis]

Cool, I was about to start working on a draft PR for this

It sounds like we just need to start composing hooks.

Yes, just in a way that makes sense from a naming and maintenance perspective

[ekzyis]

I decided to do a mix of idea 2 and 3 in #1642 now.

I did not rename useWallets but simply replaced useWalletPayment with a function that wraps useWallets (instead of useWallet as before). However, it does not return an array but it returns a single payment function that includes the wallet loop.

Therefore, I am going to close this PR since I no longer need changes to useWallets for sender fallbacks.