Another attempt at `max_satisfaction_weight` fixes #476

evanlinjin · 2022-10-09T13:39:03Z

Replaces #474, refer to #474 (comment)

This PR has two intentions:

Redefine max_satisfaction_weight to be the difference in TxIn weight between "satisfied" and "unsatisfied" states. In an "unsatisfied" state, we still need to include the scriptSigLen varint, as well as the witnessStackLen (for txs with at least one segwit spend).
Attempt further fixes to improve accuracy of max_satisfaction_weight.

Comments, tests and examples have been updated to reflect the above intentions.

Notes for reviewers

The new definition of max_satisfaction_weight can be seen in this comment:

rust-miniscript/src/descriptor/mod.rs

Lines 320 to 339 in 08cff39

    
               /// Computes an upper bound on the difference in weight between a 
        
               /// non-satisfied `TxIn` and a satisfied `TxIn`. 
        
               /// 
        
               /// A non-satisfied `TxIn` contains the `scriptSigLen` varint recording the 
        
               /// value 0 (which is 4WU) and also the `witnessStackLen` for transactions 
        
               /// that have at least one witness spend, which will also record the value 0 
        
               /// (which is 1WU). 
        
               /// 
        
               /// Hence, "satisfaction weight" contains the additional varint weight of 
        
               /// `scriptSigLen` and `witnessStackLen` (which occurs when the varint value 
        
               /// increases over the 1byte threshold), and also the weights of `scriptSig` 
        
               /// and the rest of the `witnessField` (which is the stack items and the 
        
               /// stack-item-len varints of each item). 
        
               /// 
        
               /// This assumes a ec-signatures is always 73 bytes, and the sighash prefix 
        
               /// is always included (+1 byte). 
        
               /// 
        
               /// # Errors 
        
               /// When the descriptor is impossible to safisfy (ex: sh(OP_FALSE)). 
        
               pub fn max_satisfaction_weight(&self) -> Result<usize, Error> {

evanlinjin · 2022-10-10T02:17:20Z

@LLFourn A review when you have time would be appreciated 🙏

LLFourn

Nice work. I think this is a coherent and useful definition of max_satisfaction_weight. I was actually beginning to wonder if this was even possible.

Not sure if it's wise to simply change it under people's noses. Everyone who was using max_satisfaction_weight before was probably making a mistake or two but this breaking change seems likely to lead to undershooting of fees even when this would only happen reviously when they were doing something weird like adding segwit and non-segwit coins at the same time.

Maybe this should be a new method like max_weight_to_satisfy or something less awkward and deprecate max_satisfaction_weight in favor of it and stressing the need to read the documentation.

LLFourn · 2022-10-10T04:38:21Z

src/descriptor/sh.rs

-                4 * (varint_len(scriptsig_len) + scriptsig_len)
+                let scriptsig_size = ps + ss + ms.max_satisfaction_size()?;
+                let scriptsig_varint_diff = varint_len(scriptsig_size) - varint_len(0);
+                4 * (scriptsig_varint_diff + scriptsig_size)


Prefer just return the scriptsig_size from each branch and do the final weight calculation once outside.

LLFourn · 2022-10-10T04:39:08Z

src/descriptor/tr.rs

-    /// Assumes all ec-signatures are 73 bytes, including push opcode and
-    /// sighash suffix. Includes the weight of the VarInts encoding the
-    /// scriptSig and witness stack length.
+    /// Assumes a ec-signature is 73 bytes and the sigHash suffix is always


nit: s/ec-signature/ECDSA signature/

LLFourn · 2022-10-10T04:40:21Z

src/miniscript/context.rs

@@ -222,6 +222,7 @@ where

    /// Depending on script context, the size of a satifaction witness may slightly differ.
    fn max_satisfaction_size<Pk: MiniscriptKey>(ms: &Miniscript<Pk, Self>) -> Option<usize>;
+


LLFourn · 2022-10-10T04:54:30Z

src/lib.rs

+//! // = 1 + (1 + 33 + 1 + 1 + 1 + 1 + 33 + 1 + 1) = 74 WU
+//! // stackItem[Sig]: varint <sig+sighash> = 1 + 72+1 = 74 WU (TODO: Figure out why sig is 72bytes here)
+//! // Expected satisfaction weight: 136 + 74 + 74 = 284
+//! assert_eq!(desc.max_satisfaction_weight().unwrap(), 284);


why did this drop by 9 WU. I would have expected it only to drop by 5 (no scriptSig and witnessStcklen)?

I'm pretty sure the original calculations were wrong. I recalculated in the comments, but let me know if I've missed something.

But it was asserted in the doctest so they should have run in CI and reflect what the previous version did. Something else must be going on...

You're missing a OP_34 at the start of the scriptSig both here and in the max_satisfaction_weight code (I'm pushing the fix soon ™️)

You can see a tx spending from a similar descriptor (keys are different) here: https://blockstream.info/testnet/tx/81c27770d921534b93d4172d6673ad8aca1e8212ead6f6ed4d545aa9b37b243c?expand

evanlinjin · 2022-11-11T08:55:10Z

@apoelstra Could you please have a look at this PR when you have time? Would love to know your thoughts on these changes and proposed definition change of "satisfaction weight". Thank you in advance!

danielabrozzoni

I have a couple of nits on wording, I just did a quick scroll, I didn't look at the code in deep.

I'm not fond of changing the definition of max_sat_weight, but I think it'd be great to implement LLoyds' suggestion of having a new method (max_weight_to_satisfy) instead.

After a chat with @evanlinjin, we decided that I'll take over this PR, as he has a lot on his plate already :). I'm going to wait for some feedback on max_weight_to_satisfy (or however we want to call it), and then go over the comments and update the code accordingly

danielabrozzoni · 2022-11-11T15:29:21Z

src/descriptor/bare.rs

-    /// # Errors
-    /// When the descriptor is impossible to safisfy (ex: sh(OP_FALSE)).


Why is the Errors section being removed?

danielabrozzoni · 2022-11-11T15:30:59Z

src/descriptor/mod.rs

+    /// and the rest of the `witnessField` (which is the stack items and the
+    /// stack-item-len varints of each item).
+    ///
+    /// This assumes a ec-signatures is always 73 bytes, and the sighash prefix


nit: This assumes a ec-signatures are always 73 bytes

a should also be that :)

danielabrozzoni · 2022-11-11T15:35:39Z

src/descriptor/mod.rs

+    /// stack-item-len varints of each item).
+    ///
+    /// This assumes a ec-signatures is always 73 bytes, and the sighash prefix
+    /// is always included (+1 byte).


nit: I liked the old comment a bit better. From the old comment, it seemed that 73 bytes included the push opcode and the sighash suffix (making the actual signature 71 bytes). From this new version, it seems that ec signatues are assumed to be 73 bytes +1 byte for the sighash = 74 bytes

(But maybe it's just me)

apoelstra · 2022-11-14T14:38:27Z

Thanks @danielabrozzoni !

I agree with all of your nits. ACK 238014a otherwise.

Definitely agree that we should keep the old method, deprecate it, and make a new one with a new name. This is fine to go into 9.1 (since I am going to try to release 9.0 ASAP, see #488 ). I have no opinion on name. max_weight_to_satisfy sounds fine to me.

danielabrozzoni · 2022-11-21T11:05:02Z

I just pushed 734acee1fb941ba283e79c0603d413a4e0f4ac0f:

rebased
deprecated max_satisfaction_weight and introduced max_weight_to_satisfy instead, updated examples accordingly
fixed nits in docs
fixed some bugs in this PR's code (see Another attempt at max_satisfaction_weight fixes #476 (comment), and also the same bug was in the p2sh-p2wpkh case)

apoelstra

ACK 734acee

LLFourn · 2022-11-23T02:51:18Z

src/descriptor/mod.rs

+    ///
+    /// Hence, "satisfaction weight" contains the additional varint weight of
+    /// `scriptSigLen` and `witnessStackLen` (which occurs when the varint value
+    /// increases over the 1byte threshold), and also the weights of `scriptSig`


So what this is wants to say about scriptSigLen and witnessStackLen is:

The witnessStackLen is is assumed to be 1 for segwit descriptors but 0 for legacy

The scriptSigLen is always by default 1 regardless.

This function will tell you how much satisfying the input will increase these over these assumed defaults.

I wonder if is possible to make this clearer.

I made this issue at rust-bitcoin/rust-bitcoin#1411 where it seems I was mistaken about the definition we were using here. I thought we were doing weight over the weight of TxIn::default() but we are doing one less than that for segwit descriptors since we are use 1 as a base witness stack len where as it would be 0 for TxIn::default(). What do you think about changing the definition to simply the weight over TxIn::default() once it is satisfied (assuming Txin::weight existed) i.e. this always holds:

for i in 0..transaction.input.len() { assert_eq!(descriptor_for_input[i].max_weight_to_satisfy(), transaction.input[i].weight() - Txin::default().weight()_ }

in the case where transaction is valid and you used the worst satisfaction path from the corresponding input in descriptor_for_input.
This might be a bit easier to explain (assuming TxIn::weight is coherent and easy to explain itself!).

What do you think about changing the definition to simply the weight over TxIn::default() once it is satisfied (assuming Txin::weight existed) i.e. this always holds:

I think this is a good idea. I also tripped over the "starting from 1" logic here. It does make sense, and presumably wallet authors want us to return the smallest possible number that is strictly true (or maybe they want us to return the most predictable and reproducible number, so if this is used for validity checks we won't get inconsistencies like the one that crashed Liquid?), so I let it slide ... but IMO it's a bit surprising and confusing.

Maybe the right way to think about this function is that it's used for sanity/validity checking, and we want it to be a simple-to-explain value which tries not to do ad-hoc computations that feel likely to change between version of the library.

When wallets actually want to do fee estimation at spend-time, they presumably have more information and can use the planning API #481 and we can put as many accounting tricks as we want into that.

Thanks for the feedback! I think you're right, having a coherent definition of TxIn and basing the max_weight_to_satisfy calculation on that sounds less error-prone for the end user. I'll open a PR on rust-bitcoin soon :)

From my understanding, @LLFourn is not suggesting to change the implementation of max_weight_to_satisfy, but to change how we word things to make it clearer to the end user.

So we assume TxIn::default()'s weight always includes 1 byte for len(witness_stack_size), and max_weight_to_satisfy just becomes "<satisfied_txin>.weight() - TxIn::default().weight()`.

But if we assume at least one input is a segwit spend (which is also the "safer" definition), the current implementation can be worded as <miniscript_thing>.max_weight_to_satisfy() - TxIn::default().max_weight(), where TxIn::max_weight assumes segwit spend.

That's not what the current implementation does if I am not mistaken. It assumes 0 witness stack len for non segwit descriptor and 1 for a segwit descriptor.

But if we assume at least one input is a segwit spend (which is also the "safer" definition)

Why is this safer though. What guarantee does doing this provide? It doesn't guarantee that if you add all the max_weight_to_satisfy of all the inputs together that the sum is an upper bound on the weight contribution of the inputs. You have to know whether one input was segwit to know whether to add the 2WU segwit header. Since you always need to figure out whether there is a segwit input anyway then you might as well use that information to add +1 WU for all non-segwit inputs while you're at it. This gives you a correct upper bound on the input weight (if you add the weight of the length byte for the number of inputs).

By assuming that the witness stack length is always 1WU for non-segwit inputs you can still get the correct upper bound so it's also a decent choice. You just have to go and subtract 1WU from all the non-segwit inputs in the case that all of them are non-segwit.

As @apoelstra points out there is no great answer here. We should just try and choose the easiest thing to explain correctly. To me the concept of TxIn::weight while not elegant is coherent enough to act as a focal point of this definition. Your explanation involving Txin::max_weight seems worse in this respect since what does the max weight of a TxIn even mean?

That's not what the current implementation does if I am not mistaken. It assumes 0 witness stack len for non segwit descriptor and 1 for a segwit descriptor.

~~Yep, correct~~ (uhm, or maybe not, see Evan's comment)

Since you always need to figure out whether there is a segwit input anyway then you might as well use that information to add +1 WU for all non-segwit inputs while you're at it.

(Note that this is not always possible to do: if you're creating a tx spending some P2SH utxos that aren't yours, you can't know before the other party signs if they're legacy or segwit wrapped, and max_weight_to_satisfy is often used before signing. But in case you don't you can just assume segwit, and you're fine)

I think we all agree that there's no way to produce the right result without the need for a "second pass" after the whole transaction is made.

Since I believe most users will forget to do that next step (and many may not care to do it at all) I think it's better if we return something that can be used "safely" straight away, even if it means potentially overshooting the fee. So always assuming segwit seems the best choice to me.

More advanced users could then check the tx and subtract -1 * n_inputs if the tx is non-segwit. We can provide an example for that and try to document it the best we can.

@LLFourn I'm a little confused by what you mean, so to make sure we are on the same page I will give thoughts to your original comment...

The witnessStackLen is assumed to be 1 for segwit descriptors but 0 for legacy

From my understanding this is not true. The definition of max_weight_to_satisfy is the additional weight introduced from satisfying a TxIn. For satisfying legacy descriptors, we will never add additional items to the witness stack, so the additional weight of witnessStackLen for legacy descriptors is 0.

This is independent to whether TxIn::default() (an unsatisfied txin) contains the witnessStackLen field.

@LLFourn I'm a little confused by what you mean, so to make sure we are on the same page I will give thoughts to your original comment...

The witnessStackLen is assumed to be 1 for segwit descriptors but 0 for legacy

From my understanding this is not true. The definition of max_weight_to_satisfy is the additional weight introduced from satisfying a TxIn. For satisfying legacy descriptors, we will never add additional items to the witness stack, so the additional weight of witnessStackLen for legacy descriptors is 0.

Correct. I don't think what you said is in contradiction to what I said.

But if we assume at least one input is a segwit spend (which is also the "safer" definition), the current implementation can be worded as <miniscript_thing>.max_weight_to_satisfy() - TxIn::default().max_weight(), where TxIn::max_weight assumes segwit spend.

Ok now I understand what you were saying here and I think we are in the same page. I still think this definition is awkward though (I was at least confused by it!). I think what @danielabrozzoni is suggesting would be ok for this function. I do think that #481 should use the definition I gave (weight over TxIn::default) and not add the extra weight unit for legacy.

evanlinjin · 2022-12-06T08:41:02Z

src/descriptor/segwitv0.rs

+        };
+        // stack size varint difference between non-satisfied (0) and satisfied
+        // `max_sat_elems` is inclusive of the "witness script" (redeem script)
+        let stack_varint_diff = varint_len(max_sat_elems) - varint_len(0);


If max_satisfaction_witness_elements does not include the "redeem script" item (which I assume it doesn't...

Suggested change

let stack_varint_diff = varint_len(max_sat_elems) - varint_len(0);

let stack_varint_diff = varint_len(max_sat_elems + 1) - varint_len(0);

Seems like max_satisfaction_witness_elements includes the redeem script, as the docs say "Maximum number of witness elements used to satisfy the Miniscript fragment, including the witness script itself."

danielabrozzoni · 2022-12-14T19:29:26Z

I opened rust-bitcoin/rust-bitcoin#1467 to introduce TxIn::segwit_weight() and TxIn::legacy_weight(), then pushed here a rewording of the definition of max_satisfaction_weight, which hopefully is clearer.

For segwit inputs or legacy inputs included in segwit txs, it holds:

for i in 0..transaction.input.len() {
    assert_eq!(descriptor_for_input[i].max_weight_to_satisfy(), transaction.input[i].segwit_weight() - Txin::default().segwit_weight())
}

Instead, for legacy transactions, it holds:

for i in 0..transaction.input.len() {
    assert_eq!(descriptor_for_input[i].max_weight_to_satisfy(), transaction.input[i].legacy_weight() - Txin::default().legacy_weight())
}

Which I think is pretty neat :)

Once rust-bitcoin/rust-bitcoin#1467 is merged and miniscript is up-to-date with the latest rust-bitcoin, I can add a test to verify that the two invariants above hold.

LLFourn · 2022-12-16T03:58:59Z

src/descriptor/mod.rs

+    /// Since this method uses `segwit_weight` instead of `legacy_weight`,
+    /// if you want to include only legacy inputs in your transaction,
+    /// you should remove 1WU from each input's `max_weight_to_satisfy`
+    /// for a more accurate estimate.


paragraph duplicated.

LLFourn · 2022-12-16T04:06:18Z

src/descriptor/mod.rs

+    ///         descriptor_for_input[i].max_weight_to_satisfy(),
+    ///         transaction.input[i].segwit_weight() - Txin::default().segwit_weight()
+    ///     );
+    /// }


I think you need specify something like this:

/// In other words, for segwit inputs or legacy inputs included in
/// segwit transactions, the following will hold for each input if that input was satisfied with the largest possible witness.

It would be nice to have some tests against random descriptors that checks this does actually hold. Do we have a method of doing proptests like this already?

Thanks, updated!

It would be nice to have some tests against random descriptors that checks this does actually hold.

I agree that it would be nice to have some tests, but unfortunately we have to wait for the newest rust-bitcoin to be released and for rust-miniscript to be updated before being able to write any.

Do we have a method of doing proptests like this already?

I'm not entirely sure, I guess I could expand tests/test_desc.rs

Ah right. Tests for this equality will have to wait I guess.

Re proptests, we have a foray into Kani in rust-bitcoin rust-bitcoin/rust-bitcoin#1415 but I doubt Kani (which does model-checking using a SMT solver) could handle stuff here, unfortunately.

LLFourn · 2022-12-16T04:07:37Z

New definition and docs really makes a lot of sense. I think as close to making sense as you can when trying to explain this stuff. I couple of nits but otherwise utACK

apoelstra · 2022-12-19T19:12:26Z

It looks like this new version uses "ec-signatures" everywhere rather than "ECDSA`. I skimmed through the earlier discussion and didn't see, so I hope I'm not relitigating stuff, but I'm pretty sure that "ECDSA" is correct in every instance. Schnorr sigs are also EC sigs, and they are always 64/65 bytes.

Anyway I won't hold up this PR over that, it's not a huge deal.

apoelstra

ACK f3c7ad8

danielabrozzoni · 2022-12-19T21:24:40Z

It looks like this new version uses "ec-signatures" everywhere rather than "ECDSA`. I skimmed through the earlier discussion and didn't see, so I hope I'm not relitigating stuff, but I'm pretty sure that "ECDSA" is correct in every instance. Schnorr sigs are also EC sigs, and they are always 64/65 bytes.

I'm using "ECDSA signatures" almost everywhere (there's still one instance of "ec signatures" which I missed in src/descriptor/sh.rs, my bad), saying:

    /// Assumes all ECDSA signatures are 73 bytes, including push opcode and
    /// sighash suffix.
    /// Assumes all Schnorr signatures are 66 bytes, including push opcode and
    /// sighash suffix.

If you prefer to wait for tests before merging this, I can remove the "ec signatures" from sh.rs as well, otherwise, I can fix in the PR adding the tests, as you wish :)

apoelstra · 2022-12-19T21:26:54Z

I'm going to wait on @sanket1729 to merge this since it's non-trivial and he's been the primary maintainer of the library lately. I don't know what his timeline is, but I think you have time to add tests and tweak the docs :). I can quickly re-ack any minor changes.

sanket1729

ACK f3c7ad8

sanket1729 · 2022-12-31T18:34:45Z

ACK modulo requiring rebase

This commit has two intentions: 1. Define `max_weight_to_satisfy` to be the difference between a "satisfied" TxIn's `segwit_weight` and an "unsatisfied" TxIn's `segwit_weight` 2. Deprecrate `max_satisfaction_weight` Comments, tests and examples have been updated to reflect the above intentions. Co-authored-by: Daniela Brozzoni <[email protected]>

danielabrozzoni · 2022-12-31T19:25:40Z

Rebased

sanket1729

ACK 7296f8e

sanket1729 · 2022-12-31T20:16:43Z

Let's the new year with a bang!

evanlinjin · 2023-01-01T02:21:06Z

Thank you @danielabrozzoni for taking this forward!

7fc7661 Deprecate across the board max_satisfaction_weight (Jose Storopoli) Pull request description: Adds a `since=10.0.0` to all `max_satisfaction_weight` deprecations. Adds a note telling users to check #476 for more details. Closes #637. ACKs for top commit: apoelstra: ACK 7fc7661 tcharding: ACK 7fc7661 Tree-SHA512: 90a75bd44d5b0bec5044fc58186323f6c992e43958a912d9d36a1bda411ef6156076ac2125ee6dc8806a742b0aef046ae1f540911301972c8c2f95bb02ec8980

evanlinjin mentioned this pull request Oct 9, 2022

Stack len for legacy max_satisfaction_weight calculations #474

Closed

evanlinjin force-pushed the fix-max-satisfaction-weight-definition branch 2 times, most recently from 08cff39 to 07a51f7 Compare October 9, 2022 13:56

evanlinjin mentioned this pull request Oct 9, 2022

FeeRate calculation fixes bitcoindevkit/bdk#774

Closed

4 tasks

evanlinjin force-pushed the fix-max-satisfaction-weight-definition branch from 07a51f7 to 238014a Compare October 9, 2022 14:18

LLFourn reviewed Oct 10, 2022

View reviewed changes

evanlinjin mentioned this pull request Oct 20, 2022

Fix max_satisfaction_weight calculations for taproot. #473

Merged

danielabrozzoni reviewed Nov 11, 2022

View reviewed changes

danielabrozzoni force-pushed the fix-max-satisfaction-weight-definition branch 2 times, most recently from abeda65 to 734acee Compare November 21, 2022 10:58

apoelstra previously approved these changes Nov 22, 2022

View reviewed changes

LLFourn reviewed Nov 23, 2022

View reviewed changes

LLFourn mentioned this pull request Nov 23, 2022

Add .weight() for transaction components rust-bitcoin/rust-bitcoin#1411

Closed

evanlinjin commented Dec 6, 2022

View reviewed changes

danielabrozzoni dismissed apoelstra’s stale review via ff107bd December 13, 2022 14:13

danielabrozzoni force-pushed the fix-max-satisfaction-weight-definition branch 2 times, most recently from ff107bd to ae5bdaf Compare December 14, 2022 19:29

danielabrozzoni mentioned this pull request Dec 14, 2022

Add weight utilities to TxIn and TxOut rust-bitcoin/rust-bitcoin#1467

Merged

LLFourn reviewed Dec 16, 2022

View reviewed changes

danielabrozzoni force-pushed the fix-max-satisfaction-weight-definition branch from ae5bdaf to f3c7ad8 Compare December 18, 2022 19:17

apoelstra previously approved these changes Dec 19, 2022

View reviewed changes

sanket1729 previously approved these changes Dec 31, 2022

View reviewed changes

danielabrozzoni dismissed stale reviews from sanket1729 and apoelstra via 7296f8e December 31, 2022 19:06

danielabrozzoni force-pushed the fix-max-satisfaction-weight-definition branch from f3c7ad8 to 7296f8e Compare December 31, 2022 19:06

sanket1729 approved these changes Dec 31, 2022

View reviewed changes

sanket1729 merged commit c08f6dc into rust-bitcoin:master Dec 31, 2022

realeinherjar mentioned this pull request Sep 19, 2023

fix: remove deprecated max_satisfaction_weight bitcoindevkit/bdk#1115

Closed

6 tasks

apoelstra mentioned this pull request Feb 15, 2024

max_satisfaction_weight is only deprecated for some descriptors #637

Closed

storopoli mentioned this pull request Feb 15, 2024

Deprecate across the board max_satisfaction_weight #638

Merged

RCasatta mentioned this pull request Mar 1, 2024

use max_weight_to_satisfy instead of deprecated max_satisfaction_weight #647

Closed

	/// Computes an upper bound on the difference in weight between a
	/// non-satisfied `TxIn` and a satisfied `TxIn`.
	///
	/// A non-satisfied `TxIn` contains the `scriptSigLen` varint recording the
	/// value 0 (which is 4WU) and also the `witnessStackLen` for transactions
	/// that have at least one witness spend, which will also record the value 0
	/// (which is 1WU).
	///
	/// Hence, "satisfaction weight" contains the additional varint weight of
	/// `scriptSigLen` and `witnessStackLen` (which occurs when the varint value
	/// increases over the 1byte threshold), and also the weights of `scriptSig`
	/// and the rest of the `witnessField` (which is the stack items and the
	/// stack-item-len varints of each item).
	///
	/// This assumes a ec-signatures is always 73 bytes, and the sighash prefix
	/// is always included (+1 byte).
	///
	/// # Errors
	/// When the descriptor is impossible to safisfy (ex: sh(OP_FALSE)).
	pub fn max_satisfaction_weight(&self) -> Result<usize, Error> {

		@@ -222,6 +222,7 @@ where

		/// Depending on script context, the size of a satifaction witness may slightly differ.
		fn max_satisfaction_size<Pk: MiniscriptKey>(ms: &Miniscript<Pk, Self>) -> Option<usize>;

	let stack_varint_diff = varint_len(max_sat_elems) - varint_len(0);
	let stack_varint_diff = varint_len(max_sat_elems + 1) - varint_len(0);

Another attempt at max_satisfaction_weight fixes #476

Another attempt at max_satisfaction_weight fixes #476

Conversation

evanlinjin commented Oct 9, 2022 • edited Loading

Notes for reviewers

evanlinjin commented Oct 10, 2022

LLFourn left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

evanlinjin commented Nov 11, 2022

danielabrozzoni left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

apoelstra commented Nov 14, 2022

danielabrozzoni commented Nov 21, 2022

apoelstra left a comment

Choose a reason for hiding this comment

LLFourn Nov 23, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

danielabrozzoni Dec 6, 2022 • edited Loading

Choose a reason for hiding this comment

evanlinjin Dec 6, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LLFourn Dec 6, 2022 • edited Loading

Choose a reason for hiding this comment

danielabrozzoni Dec 6, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LLFourn Dec 6, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

danielabrozzoni commented Dec 14, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

danielabrozzoni Dec 18, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LLFourn commented Dec 16, 2022

apoelstra commented Dec 19, 2022

apoelstra left a comment

Choose a reason for hiding this comment

danielabrozzoni commented Dec 19, 2022

apoelstra commented Dec 19, 2022

sanket1729 left a comment

Choose a reason for hiding this comment

sanket1729 commented Dec 31, 2022

danielabrozzoni commented Dec 31, 2022

sanket1729 left a comment

Choose a reason for hiding this comment

sanket1729 commented Dec 31, 2022

evanlinjin commented Jan 1, 2023

Another attempt at `max_satisfaction_weight` fixes #476

Another attempt at `max_satisfaction_weight` fixes #476

evanlinjin commented Oct 9, 2022 •

edited

Loading

LLFourn left a comment •

edited

Loading

LLFourn Nov 23, 2022 •

edited

Loading

danielabrozzoni Dec 6, 2022 •

edited

Loading

evanlinjin Dec 6, 2022 •

edited

Loading

LLFourn Dec 6, 2022 •

edited

Loading

danielabrozzoni Dec 6, 2022 •

edited

Loading

LLFourn Dec 6, 2022 •

edited

Loading

danielabrozzoni Dec 18, 2022 •

edited

Loading