Add effective value calculation

[yancyribbens]

Draft PR for adding effective value calculation to TxOut. Adding this method was discussed here: #2217

[yancyribbens]

@Kixunil what do you think? Will add some tests around this if it seems ok.

[yancyribbens]

force push to add test for effective_value

[Kixunil]

Thinking about this it's weird that it's a method on TxOut since script_pubkey is not used. It kinda should be on Amount but that seems odd too. Perhaps have a free-standing function in the transaction module that takes just Amount (and the remaining parameters`)?

If there's a method on TxOut it should really do something with the script - e.g. scan it and determine that it's P2PKH and use known constant. Return None if it's unknown script. But then the question is what to do about P2TR - it's not possible to know if it will be a script spend or key spend.

I think there should be something like SpendInfo type that carries information about how an output can be spent and have a method on it. But this is an advanced idea and would need a lot of design work etc. Also I guess something like this is already covered by miniscript. So while I'm intrigued by it I prefer to postpone it. We have ton of other more urgent work now.

[yancyribbens]

So while I'm intrigued by it I prefer to postpone it. We have ton of other more urgent work now.

Ok, well in the meantime re-opening this PR #2217 then which I had closed in favor of this current work would fix the broken crate I'm working on. Any opposition to that?

[Kixunil]

I mean I'm intrigued by the advanced stuff. A free-standing function would be still fine (and still useful for uncommon cases like custom scripts).

[yancyribbens]

I mean I'm intrigued by the advanced stuff. A free-standing function would be still fine (and still useful for uncommon cases like custom scripts).

Ah ok, I thought you where second guessing if effective_value is worth adding to rust-bitcoin. It sounds like for the near-term, the question is if it should be part of TxOut or a helper function else where?

[Kixunil]

Yeah, I think it should be a free-standing function.

[yancyribbens]

Yeah, I think it should be a free-standing function.

Sure, but where? I mean it does need self.value from TxOut so it doesn't feel completely un-natural to go here, but yeah, could also just take value as a param.

[Kixunil]

In the transaction module.

[yancyribbens]

Rebased and dropped a clippy commit that had already been merged with master.

[yancyribbens]

I added : to effective_value can be calculated as: value - (fee_rate * weight).. Also waffling back and forth on if this is describing a TxOut or the TxIn to a new transaction.

[Kixunil]

ACK f2b992c

[yancyribbens]

Composing an input set for a transaction requires the input amount to be the net amount of spending the corresponding output (TxOut), so I believe the terminology TxOut here is correct.

[yancyribbens]

Sorry for all the noise. Should read "value of a output" not input which is what it was before.

[yancyribbens]

changed to an output instead of a output (grammar).

[yancyribbens]

Rebased with master

[yancyribbens]

Maybe I ought to reword the doc comment since it's sort of confusing how the commit uses the TxIn::BASE_WEIGHT.

What about something like:

The effective_value is used to calculate how much a particular output contributes to a target spend amount. Therefore, the effective_value returns the net amount, that is, the output amount minus the overhead to spend the output. The overhead to spend the output is the fee. In order to calculate the fee, the size of the input which consumes the output is calculated as the input base_weight plus the size of the output once the spending conditions are satisfied (the satisfaction_weight) multiplied by a fee_rate. Lastly, subtract the fee from the output value to find the effective_value.

[yancyribbens]

If we incorporate the output version of this API, the effective cost of adding a change output.

@sanket1729 I'm not sure I follow. Can you give an example of how using the effective cost of a change output is used?

[sanket1729]

@yancyribbens, I have seen some coin selection code where the cost of a change output is subtracted from the change output. It is also useful when building complex transactions with multiple recipients.

In any case, we don't need to add this as there is no requirement for this.

[sanket1729]

ACK 4c753c8

[yancyribbens]

I have seen some coin selection code where the cost of a change output is subtracted from the change output. It is also useful when building complex transactions with multiple recipients.

Ah ok, I see. I'm starting to 2nd guess if this is actually worth adding to Rust Bitcoin since it's so specific to coin selection. Maybe it's better to just keep this within rust-bitcoin-coin-selection instead of rust-bitcoin.

[Kixunil]

ACK d69d628

[apoelstra]

ACK d69d628