Thanks for the explanation in the PR. I like your approach, but I'm wondering if this will still be accurate about create_self_transfer:
"""Create and return a tx with the specified fee_rate. Fee may be exact or at most one satoshi higher than needed."""
Because right now, using a fee rate of Decimal("0.003") the P2PK tx will have a fee of Decimal('0.0005025'), but the exact fee rates should either be
Decimal('0.000504') for 168 vbytes, or
Decimal('0.000501') for 167 vbytes

So it seems like the fee rate won't be exact anymore unless i'm missing something. I feel like it might be a problem for some tests if create_self_transfer created transactions where the fee rate wasn't exactly the same as what was specified.

It's probably not an issue when no fee rate has been specified, but perhaps in the case where the user does specify a fee rate then the signature could be restricted to 71 bytes?

@DariusParvin: Thanks for reviewing, you brought up an interesting point here.

Indeed, the promise "Fee may be exact or at most one satoshi higher than needed." does currently not apply for MiniWallet transaction created in P2PK mode (and on the master branch it's obviously totally off, since the vsize was not set properly). As far as I can tell the only way to achieve this would be with a constant signature size, otherwise we have a chicken and egg situation: for creating a signature we need to have the output amount already set (which determines the fee), but for a achieving a certain fee-rate we need to know the transaction size which is not known until the signature is created.

So it seems like the fee rate won't be exact anymore unless i'm missing something. I feel like it might be a problem for some tests if create_self_transfer created transactions where the fee rate wasn't exactly the same as what was specified.

Good question, I don't know enough of the "big picture" of all tests to know whether having exact fees is something that is really needed (and if it really is, if the anyone-can-spend modes are not sufficient then?).

I could generally think of two possible solutions:

• Keep the current logic in the PR and only adapt the comment (e.g. something like "For anyone-can-spend outputs, the fee may be exact or at least most one satoshi higher than needed. For P2PK outputs, the fee is approximated due to varying signature size.")
• Grind signatures to always have fixed size (potentially slow since .sign_ecdsa has to be executed about twice on average to get a single signature), as you suggested:

It's probably not an issue when no fee rate has been specified, but perhaps in the case where the user does specify a fee rate then the signature could be restricted to 71 bytes?

Since the fee_rate is a parameter with default value, I don't think we can tell whether the user specified an explicit fee rate or not (at least not with the current interface of create_self_transfer()). What I could think of though is introducing a parameter to sign_tx that specifies if the signature should be forced to have a fixed size (false by default), and setting it to true in the create_self_transfer() method. That maybe is a good compromise: for transaction created with the wallet, the fee is calculated exactly, for external uses of .sign_tx the signature size and fee don't matter, since the transaction probably doesn't end up in the mempool anyway, but only in blocks.

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

Conflicts

No conflicts as of last run.

@theStack

What I could think of though is introducing a parameter to sign_tx that specifies if the signature should be forced to have a fixed size (false by default), and setting it to true in the create_self_transfer() method. That maybe is a good compromise: for transaction created with the wallet, the fee is calculated exactly, for external uses of .sign_tx the signature size and fee don't matter, since the transaction probably doesn't end up in the mempool anyway, but only in blocks.

I think that's a reasonable compromise. The only thing is I think it might be better to have the default be True instead, mainly because the fee rate is quite important for a few tests (e.g. getting included in a block, replaced in the mempool, fee limits) and it could be very hard to debug if the fee rate is random between test runs.

Rebased on master, changed sign_tx to create fixed-size signatures (71 bytes) by default (as proposed by DariusParvin), force-pushed and updated the PR description.

@theStack

What I could think of though is introducing a parameter to sign_tx that specifies if the signature should be forced to have a fixed size (false by default), and setting it to true in the create_self_transfer() method. That maybe is a good compromise: for transaction created with the wallet, the fee is calculated exactly, for external uses of .sign_tx the signature size and fee don't matter, since the transaction probably doesn't end up in the mempool anyway, but only in blocks.

I think that's a reasonable compromise. The only thing is I think it might be better to have the default be True instead, mainly because the fee rate is quite important for a few tests (e.g. getting included in a block, replaced in the mempool, fee limits) and it could be very hard to debug if the fee rate is random between test runs.

@DariusParvin: After repeatedly executing the test feature_csv_activation.py (the only one so far making extensive use of sign_tx()), I didn't see significant differences in execution time, so I guess grinding to a fixed size is okay. I agree that in a test framework it is preferrable to have reproducible results rather than squeezing out a little less execution time. If that is a concern in future test that create lots of signatures, the parameter fixed_length can be set to False explicitely.

Concept ACK d6d2ab9