feat: scheduled transactions prototype

[ghost]

This is a bare minimum MVP-style integration of scheduled transactions.

Before we merge this, we'll have to switch the backend repo back to the upstream joinmarket-clientserver repo. We'll be able to do that once the API support for scheduled transactions is released which is scheduled (😅) to be included the next Joinmarket release. Until then I'll keep this PR on draft. It is ready for review though.

Give it a spin and let me know how it goes. UI is obviously something that'll change quite a lot in upcoming versions. It's probably okay for a very first version though. Just to get the foot in the door so to speak and have something to iterate on.

Let me know what you think! I'll work on progress reporting in the mean time.

Also let me hear suggestions for the copy. I put something off the top of my head -- haven't really thought about it too deeply.

📸

Relevant backend changes:

• Scheduled transactions API support: Add RPC API endpoint for tumbler JoinMarket-Org/joinmarket-clientserver#1252
• Options support: Allow specifiying options for taker/schedule API endpoint JoinMarket-Org/joinmarket-clientserver#1260

[MaxHillebrand]

This is really nice.

It would be better to not have a global setting with "keep funds in wallet", but instead make this a button for each of the three addresses. This way I can set one or two outgoing addresses, and toggle the selfspend for the third.

[editwentyone]

how much time do we have until the other PR and our part will be delivered? I would try to flesh something out that is more or less final and includes all our last insights from the call.

[theborakompanioni]

Tested and works as expected in general. Really great job 💪

Since it is hidden with a feature flag, it is safe to be merged right away when the changes to the dev setup are reverted.
(This is not the same as "dev mode" - no user will every see the Jam tab for now)

These are some key takeaway while testing the PR:

Summary

• Works very good in general (or not so good on regtest as expected)
• Some minor adaptions to the UI can have big impacts. However, it can be visually improved later 👍
• Most of the issues probably won't arise on mainnet (e.g. the need to observe the logs if scheduler is "stuck")

Problems/Observations

• IRC problems after starting and stopping in quick succession
• Starting the scheduler without spendable funds (no utxos or only frozen utxos) has weird outputs
• Send page does not stop loading when scheduler is running. The problem seems to be that the api requests to "configget" are extremely slow/dont work at all.
• Needed to stop the scheduler after "Taker not continuing after receipt of orderbook" message in the logs

Suggestions

• Does it make sense to add short descriptions like "Jam is currently light on feedback – if the scheduler fails for whatever reason, it is safe to just stop and restart the service." ?
  • This is not pretty, but it at least gives the user a fair warning while the UI lacks notifications
• Do not start the scheduler without at least one unfrozen utxo?
• Prevent starting/stopping the scheduler in quick succession (prevent users from "freezing" their instance - this should massively help avoid spending time on support chats)

Nits

• "Start schedule" button can be clicked while data is loaded (if "Keep funds in Jam" is ticked).
• Balance is outdated after scheduler is stopped
• Error is not displayed (e.g. when address has a typo and 409 error is raised)

It would be better to not have a global setting with "keep funds in wallet", but instead make this a button for each of the three addresses. This way I can set one or two outgoing addresses, and toggle the selfspend for the third.

Max brought up a good suggestion too – I think it is best to keep track of these improvements in distinct issues. What do you think @MaxHillebrand ?

how much time do we have until the other PR and our part will be delivered? I would try to flesh something out that is more or less final and includes all our last insights from the call.

@editwentyone I think it is safe to say that with the main functionality now merged and released in JM v0.9.6, Jam is completely independent regarding release date. However, this PR – with or without visual improvements – already brings massive benefits to Jam. What do you think about adding UI improvements step by step in conjunction with feedback from users? This approach does have it's downsides too, but I guess it is quite difficult to come up with a "more or less final" version. If there is no "hard no" from you, I'd be in favor of merging this as is (maybe some tiny adaptions, e.g. show errors, prevent quick restarts, etc.). Keep in mind that it is still hidden behind a feature flag.

[ghost]

Thanks for the detailed review @theborakompanioni.

My suggestion for going forward with this:

• Merge this PR now as it works as intended and doesn't seem to introduce any big issues.
• Work on nits, copy, and smaller improvements like making sure that there are spendable, non-frozen funds. This is better done in separate PRs to keep discussions more focused (there's the feature flag so that's fine).
• Add a MVP progress report just to give the user some sense of how long it's going to take.
• Activate the feature and release Jam with the schedule feature and the listed improvements this or next week.
• Iterate and most likely rebuild the UX from scratch based on feedback, experience, and whatever we can come up based on our discussions from last week and discussions we'll most likely have next week.
• Iterate, iterate, iterate.

This allows us to get something out of the door quickly which we can then iterate on. I think the project is still in a state where we can comfortably release early and often and make sure we get maximum feedback and learn as much as possible while doing so.

Thoughts? cc @dergigi

[ghost]

It would be better to not have a global setting with "keep funds in wallet", but instead make this a button for each of the three addresses. This way I can set one or two outgoing addresses, and toggle the selfspend for the third.

Good point! The only tricky thing is that we have to explain to the user that he can't control the amount that goes to each destination address. Eventually we'll have to explain that anyway, though. I'll track this as improvement but not add it right away in this PR. I think that it'll be easier to think about wording and how exactly the behavior should be if we do it in a separate PR after we have one more iteration of the UI. Hope that works for you. 🤞

[ghost]

how much time do we have until the other PR and our part will be delivered? I would try to flesh something out that is more or less final and includes all our last insights from the call.

Let's iterate! Final versions never really stay final from my experience. 😉 Once we have a next iteration we can start working on it right away. There backend functionality was released yesterday so it's not a blocker for us anymore.

[theborakompanioni]

I thought the awaits are not necessary.. setFieldValue returns plain void. See formik#setfieldvalue.

BUT: If they are removed, the form does not work correctly anymore.. do you know what's wrong with that?
(To reproduce the error, enter an invalid address in the input field and tick "Keep funds in Jam")

[ghost]

Very good catch. I guess the issue is that without the await, the validate function gets triggered once for each setFieldValue call resulting it only seeing the field that was changed by the call that triggered the validation:

With the await, the validation function magically sees the values changed by the other setFieldValue calls:

So I guess it's just pure luck that it works. However, not sure if we should be relying on such undocumented behavior, though. I'll have a look to see if I can find a workaround. There's an endless thread on the Formik repo about something similar with no good conclusion unfortunately.

[theborakompanioni]

Uff.. sorry for getting you into that. Should we add a small warning as comment?
Otherwise I already know that I will try to remove them.. because I forget that they serve a purpose.

[ghost]

If we leave them then definitely with a comment. But I feel bad for using it in such an undocumented way. I'll look into it a bit more and if I can't find something I'll add a comment.

[ghost]

Ok changed it so that the form is explicitly verified again when all 3 addresses are filled out. Might be not ideal but at least works based on documented APIs only. Let me know what you think!

[theborakompanioni]

So just for reference, I managed to create a schedule that looks like this, when starting the scheduler without spendable utxos (either no utxos or all are frozen):

9223372036854775808,0.5824262267149192,8,INTERNAL,131.83,4,0
9223372036854775808,0,9,INTERNAL,20.65,16,0
9223372036854775809,0.8792818384272423,9,INTERNAL,5.57,16,0
9223372036854775809,0,8,bcrt1qcw8uqavrg20dflux9sr2mdh0ea7plv4ej7jgta,9.57,16,0
9223372036854775810,0.0732731932060331,9,INTERNAL,53.62,16,0
9223372036854775810,0.6704193556164667,8,INTERNAL,16.29,16,0
9223372036854775810,0,9,bcrt1qg90u7mal8t5lq7e8atm9l2kzpvk07k6lak2zde,50.35,16,0
9223372036854775811,0,8,bcrt1qdgww5emte4a9v9askruu3d6za80ttc5vk8fkla,60.41,16,0

First row is the "account"..

After this, the wallet cannot be opened anymore - Jam will display "No wallet loaded" from now on.

Same happens with the CLI:

root@1e7dfe3b919b:/src/scripts# ./wallet-tool.py funded.jmdat
User data location: /root/.joinmarket/
2022-05-09 10:03:07,498 [DEBUG]  rpc: getblockchaininfo []
2022-05-09 10:03:07,500 [DEBUG]  rpc: listwallets []
2022-05-09 10:03:07,500 [DEBUG]  rpc: getwalletinfo []
2022-05-09 10:03:07,513 [DEBUG]  rpc: getnewaddress []
Enter passphrase to decrypt wallet: 
Traceback (most recent call last):
  File "/src/scripts/./wallet-tool.py", line 6, in <module>
    jmprint(wallet_tool_main("wallets"), "success")
  File "/src/jmclient/jmclient/wallet_utils.py", line 1570, in wallet_tool_main
    wallet = open_test_wallet_maybe(
  File "/src/jmclient/jmclient/wallet_utils.py", line 1444, in open_test_wallet_maybe
    return open_wallet(path, mixdepth=max_mixdepth, **kwargs)
  File "/src/jmclient/jmclient/wallet_utils.py", line 1490, in open_wallet
    wallet = wallet_cls(storage, **kwargs)
  File "/src/jmclient/jmclient/wallet.py", line 1730, in __init__
    super().__init__(storage, **kwargs)
  File "/src/jmclient/jmclient/wallet.py", line 1135, in __init__
    super().__init__(storage, **kwargs)
  File "/src/jmclient/jmclient/wallet.py", line 1441, in __init__
    super().__init__(storage, **kwargs)
  File "/src/jmclient/jmclient/wallet.py", line 1998, in __init__
    self._populate_script_map()
  File "/src/jmclient/jmclient/wallet.py", line 2423, in _populate_script_map
    super()._populate_script_map()
  File "/src/jmclient/jmclient/wallet.py", line 2048, in _populate_script_map
    path = self.get_path(md, address_type, i)
  File "/src/jmclient/jmclient/wallet.py", line 2466, in get_path
    return super().get_path(mixdepth, address_type, index)
  File "/src/jmclient/jmclient/wallet.py", line 2124, in get_path
    return tuple(chain(self._get_bip32_export_path(mixdepth, address_type),
  File "/src/jmclient/jmclient/wallet.py", line 2242, in _get_bip32_export_path
    path = (self._get_bip32_mixdepth_path_level(mixdepth), address_type)
  File "/src/jmclient/jmclient/wallet.py", line 2311, in _get_bip32_mixdepth_path_level
    assert 0 <= mixdepth < 2**31
AssertionError

Guess that this must be reported to the server repo as well. Can you verify @dnlggr ?

[dergigi]

+1 to @dnlggr's suggestion. Let's do any nits and smaller improvements incrementally after this PR.

[theborakompanioni]

Scheduler takes some time to stop – "Stop" button is still clickable, ie. makes a request and subsequently will display an error, shortly before it is actually stopped.

Does not need to necessarily be fixed right away, can be an own GitHub Issue, just noted as a reference.

[ghost]

Scheduler takes some time to stop – "Stop" button is still clickable, ie. makes a request and subsequently will display an error, shortly before it is actually stopped.

Good find. I think 0afd258 should fixed this. On my machine, the backend response is instant so I can't really test if it works without artificially slowing things down. Can you do a quick check on your end and see if the issue is resolved?

[ghost]

I think we should be good to merge now (I hope I didn't miss anything). Everything that hasn't been addressed in this PR, I'll address via separate follow up PRs. I'll create issues for that and link them here.

[theborakompanioni]

Nice work 🪨 🪨 🪨