This adds support for multi-outputs URIs in the send tab.
For now, the receive tab is still limited to generating regular single-output URIs.
Depends on D15385
Differential D15250
[electrum] support parsing multi-output URIs PiRK on Jan 23 2024, 15:54. Authored by
Details
This adds support for multi-outputs URIs in the send tab. Depends on D15385 Paste the multi-output URI examples from the added test and from the bip21-ecash-additions.md document into the payto field, and check that the transactions are created as expected. Test also other examples from the test_bip21 test suite in the GUI, especially the exotic ones such as ecash:?op_return_raw=04deadbeef (takes one input randomly and creates an op_return output and a change output) and ecash:?r=http://domain.tld/page?h%3D2a8628fc2fbe (this one will not work because the bip70 url is invalid, but it should still try to resolve it and give a meaningful error message). python test_runner.py
Diff Detail
Event TimelineComment Actions make the test_parameter_pollution succeed for a good reason, add a test for inconsistent number of "amount" vs "addr" Comment Actions That's odd, I thought I rebased this on the parent commit just before landing it. Will rebase again. Comment Actions couple of issues
Everything generates fine Delete it -- OP_RETURN input remains. no blocker but probably not the behavior we want here.
Get multiple error notifications. I actually cannot click them all closed so that I can delete the bad string, because they keep popping up. Comment Actions fix the empty address case ecash:?op_return_raw=04deadbeef (if len(addresses) == 0, we cannot access addresses[0]). Add a test to the test plan and the unit tests Comment Actions I actually didn't think of interactively testing the ones that are supposed to fail. Good catch. Comment Actions automatically clear the "pay to" field when the URI is malformed, otherwise it will keep raising the same error as the text is changed while the user attempts to correct it. Comment Actions I'm trying to think of a solution, but that issue is not simple. Maybe the payment URI should be its own separate widget (that could be hidden and only appear when the user pastes a non-trivial URI into the "pay to" field), that when filled causes the other ones to become slaves to the URI widget. But are users still allowed to manually change anything in the other fields to adjust the transaction, or do they become read-only as long as the URI field is not cleared? Definitely not going to happen in this diff, and it would probably be good to have the opinion of a professional UI expert one of these days. Comment Actions In Cashtab this is handled by
the whole thing is pretty complicated, and this is in React which is optimized for implementing and testing this sort of UI issue. It's probably not a big deal to leave OP_RETURN populated if some input there was received, as the field is still present and visible to the user. Comment Actions This issue appears to exist in electrum before this diff, at least I get the same thing when I run ecash:qrh3ethkfms79tlcw7m736t38hp9kg5f7gycxeymme?amount=40.00&label=test&amount=30.00 through 5.2.11 Comment Actions
Spec does seem to be ambiguous about whether or not the initital address param is required. In Cashtab, the address param is de facto required, e.g. You can't send anything until you add the address param, and in this config it can only be added in the URI string. Entering this in Electrum-ABC works but it's pretty confusing to the user, as the input itself simply disappears after adding the params to the tx: In this case, we probably want the input to persist so we know what happened and why. I'm not sure on easiest solution to this. Probably just require bip21 strings to have an address. Comment Actions Is it possible to handle this without clearing the input? It's pretty difficult to troubleshoot a bad URI string with this behavior. The error msg appears, just says it's invalid, then clicking ok resets everything. It is better than the current behavior and possibly acceptable for electrum which is more power-user focused (again this issue exists in the prod version already and I don't think we've ever had a complaint about it). We don't want to re-engineer the whole UI for something that isn't having a real world impact. If we can keep the bad bip21 string in place without throwing constant error msgs without a major refactor, though, it is probably worth it. Comment Actions Not having any address seems to address some use cases, though. The other one is wanting someone to send some OP_RETURN data tx on chain without other XEC outputs. Not sure if this is a real-world need, but it was part of the rationale for https://github.com/Electron-Cash/Electron-Cash/pull/1127 I need to think this through. The obvious solution is to not throw an error message as a popup, but we still need to raise the reason for the failure to parse the URI to the user in some other way (some kind of status bar or a unique non-modal error message) . Comment Actions rebase on D15385, use a ShowPopupLabel (temporary popup, doesn't take the focus and does not block the editing of the Pay To field) instead of a blocking popup window. This is a minimalist change on the UI side. More things could be improved wrt to URI handling, as discussed in the review, but that would require a much deeper refactoring of the current code in the send tab. So I'll wait for some more feedback before deciding if a task is warranted. To summarize the issues that could (should?) be fixed: currently there is a single line field to paste either a destination address, a CSV formatted address, amount... multi-output text, or a payment URI. When a valid URI is pasted, or opened via a link, or passed on the command line, it is parsed and the various fields corresponding to the parameters are filled, and the URI disappears (replaced by the destination address). |