test: Return dict in MiniWallet::send_to #27640

Returning a tuple has many issues:

• If only one value is needed, it can not be indexed by name
• If another value is added to the return value, all call sites need to be updated

Bite the bullet now and update all call sites to fix the above issues.

<!--e57a25ab6845829454e8d69fc972939a-->

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

<!--021abf342d371248e50ceaed478a90ca-->

Reviews

See the guideline for information on the review process.

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

<!--174a7506f384e20aa4161008e828411d-->

Conflicts

Reviewers, this pull request conflicts with the following ones:

• #27602 (net processing: avoid serving non-announced txs as a result of a MEMPOOL message by sr-gi)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

Concept ACK

crACK faf4315c88d8c81c2ff84870bc81aef3cf719816

Concept ACK for moving away from a tuple return value. I think I'd prefer something more struct-like though, for example a typing.NamedTuple? Also, I don't see the point in returning tx, wtxid, and hex if no one is accessing them yet - can easily be added later on when needed?

I'd prefer something more struct-like

The idea is to use the same return type as the other MiniWallet methods. Not sure if it is worth it to use a named tuple. (rant: If you want compile-time type checks, you are better off using rust, instead of maiming python code with type annotations)

I don't see the point in returning tx, wtxid, and hex if no one is accessing them yet

The idea was to use the same return values as the other MiniWallet methods, but I am happy to drop this.

Code-review ACK faf4315c88d8c81c2ff84870bc81aef3cf719816

Anything left to do here?

@stickies-v want to take a final look?

Code review ACK faf4315c88d8c81c2ff84870bc81aef3cf719816

I'd have preferred doing things a bit differently but I think as per the PRs rationale, it's better to move away from the tuple return type quickly to avoid larger refactorings in the future. My comments can easily be implemented in follow-ups, if people want them.

The idea is to use the same return type as the other MiniWallet methods.

Fair enough, I hadn't considered that.

If you want compile-time type checks <...>

My main concern is readability and usability, being explicit about the structure of the dict is helpful to users imo and integrates well with IDEs. I still prefer using e.g. a namedtuple but that can be a follow-up (I don't think I feel strongly enough about it to open that PR myself, at least for now) that updates all relevant functions at the same time instead of doing it just for this one.

The idea was to use the same return values as the other MiniWallet methods, but I am happy to drop this.

If it were up to me I'd just have it return ["tx", "sent_vout"] since all 3 other fields can easily be accessed through "tx", but I see your rationale your rationale and I think it's reasonable.