New Witness struct to improve ser/de perfomance #672

RCasatta · 2021-09-30T15:11:34Z

At the moment the Witness struct is Vec<Vec<u8>>, the vec inside a vec cause a lot of allocations, specifically:

empty witness -> 1 allocation, while an empty vec doesn't allocate, the outer vec is not empty
witness with n elements -> n+1 allocations

The proposed Witness struct contains the serialized format of the witness. This reduces the allocations to:

empty witness -> 0 allocations
witness with n elements -> 1 allocation for most common cases (you don't know how many bytes is long the entire witness beforehand, thus you need to estimate a good value, not too big to avoid wasting space and not too low to avoid vector reallocation, I used 128 since it covers about 80% of cases on mainnet)

The inconvenience is having slightly less comfortable access to the witness, but the iterator is efficient (no allocations) and you can always collect the iteration to have a Vec of slices. If you collect the iteration you end up doing allocation anyway, but the rationale is that it is an operation you need to do rarely while ser/de is done much more often.

I had to add a bigger block to better see the improvement (ae86024), these are the results of the benches on my machine:

RCasatta/master_with_block
test blockdata::block::benches::bench_block_deserialize                 ... bench:   5,496,821 ns/iter (+/- 298,859)
test blockdata::block::benches::bench_block_serialize                   ... bench:     437,389 ns/iter (+/- 31,576)
test blockdata::block::benches::bench_block_serialize_logic             ... bench:     108,759 ns/iter (+/- 5,807)
test blockdata::transaction::benches::bench_transaction_deserialize     ... bench:         670 ns/iter (+/- 49)
test blockdata::transaction::benches::bench_transaction_get_size        ... bench:           7 ns/iter (+/- 0)
test blockdata::transaction::benches::bench_transaction_serialize       ... bench:          51 ns/iter (+/- 5)
test blockdata::transaction::benches::bench_transaction_serialize_logic ... bench:          13 ns/iter (+/- 0)

branch witness_with_block (this one)
test blockdata::block::benches::bench_block_deserialize                 ... bench:   4,302,788 ns/iter (+/- 424,806)
test blockdata::block::benches::bench_block_serialize                   ... bench:     366,493 ns/iter (+/- 42,216)
test blockdata::block::benches::bench_block_serialize_logic             ... bench:      84,646 ns/iter (+/- 7,366)
test blockdata::transaction::benches::bench_transaction_deserialize     ... bench:         648 ns/iter (+/- 77)
test blockdata::transaction::benches::bench_transaction_get_size        ... bench:           7 ns/iter (+/- 0)
test blockdata::transaction::benches::bench_transaction_serialize       ... bench:          50 ns/iter (+/- 5)
test blockdata::transaction::benches::bench_transaction_serialize_logic ... bench:          14 ns/iter (+/- 0)

With an increased performance to deserialize a block of about 21% and to serialize a block of about 16% (seems even higher than expected, need to do more tests to confirm, I'll appreciate tests results from reviewers)

src/util/bip143.rs

dr-orlovsky · 2021-09-30T16:21:17Z

Have you seen https://github.com/rust-bitcoin/rust-bitcoin/pull/668/files#diff-c55d0186f3d5990b5e223c1a8aa1bbe60988201c452dd13b3324e892bc33119eR749-R825 ? It should be very complimentary to this PR

RCasatta · 2021-10-01T07:25:48Z

Have you seen https://github.com/rust-bitcoin/rust-bitcoin/pull/668/files#diff-c55d0186f3d5990b5e223c1a8aa1bbe60988201c452dd13b3324e892bc33119eR749-R825 ? It should be very complimentary to this PR

I didn't but I've looked it now, I am not sure to grasp the complementarity since one it's address witness and the other is the witness stack in the inputs, could you elaborate?

dr-orlovsky · 2021-10-01T12:49:28Z

By "complementing" I mean they solve strong types issue for segwit related structures. I gave you link just to prevent double work from you if you will occasionally do that type which I already did

TheBlueMatt

This is awesome! I've historically always wanted to do this on a broader scale - think a similar "store it in serialized form" for an entire transaction/block and just store pointers to the start of various datastructures to provide accessors. Of course that could come later, but this is awesome by itself.

TheBlueMatt · 2021-10-01T17:06:20Z

src/blockdata/witness.rs

+
+    /// contains the witness Vec<Vec<u8>> serialization without the initial varint indicating the
+    /// number of elements (which is stored in len)
+    content: Vec<u8>,


If we're going down this path, does it make sense to also consider something like Bitcoin Core's "prevec" for the witness contents?

FWIW: https://docs.rs/tinyvec/1.5.0/tinyvec/enum.TinyVec.html

Yea, I mean such a thing is super-duper trivial to write.

I think a tinyvec like structure could probably be applicable broadly (like also for inputs/outpus) and as an addition to this, probably better to address in another PR

TheBlueMatt · 2021-10-01T17:07:13Z

src/blockdata/witness.rs

+
+#[derive(Clone, PartialEq, Eq, PartialOrd, Ord, Debug, Hash)]
+#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]  //TODO manually implement?
+pub struct Witness {


Does it make sense to have a "pointers to the start of each witness element" vec as well? I guess given most witnesses are like 2-5 elements long maybe its not worth it, but would be cool to add a benchmark for iterating a lightning HTLC transaction witness and just test.

Pointers to the start of each witness element would allow efficient index access but require another Vec.
I think for witness is not worth it because, as you said usually the witness is a few elements.

Maybe have offsets in SmallVec?
We could also optimistically assume that transactions are smaller than 4M bytes, so we could cram offsets into struct U24(u8, u8, u8);

Maybe have offsets in SmallVec?

#672 (comment)
Also, having offsets would allow efficient indexed access, but mutable access it's another story... In the end I don't think there are really use case requiring that. I think I would not add offsets here even with tinyvec/smallvec.

We could also optimistically assume that transactions are smaller than 4M bytes, so we could cram offsets into struct U24(u8, u8, u8);

the overall weight of the struct would not change because of padding (on most common 64 bits arch), I would leave this optimization in case we need other fields in the struct

dpc · 2021-10-01T19:51:55Z

iterator is efficient (no allocations) and you can always collect the iteration to have a Vec of slices

One allocation improves cache locality so even ignoring allocations, it's faster to go through one chunk of linear memory than jump between many.

RCasatta · 2021-10-05T13:49:47Z

Some updates:

squashed commits
added fuzzing
fix overflows found with fuzzing
documented a bit
removed the big block binary used to bench (cherry-pick 8412926 in case you want to bench)
checked some onchain witness usage, and apparently, 128 0c9300f#diff-13b19787ce1492ec1fe05a7e9cbba8bd24735ca71bb87e0e2f58f835b220a0feR76 is good enough to handle 80% of witnesses -> see https://opreturn.org/witness-stats/

RCasatta · 2021-10-12T12:43:11Z

This is awesome! I've historically always wanted to do this on a broader scale - think a similar "store it in serialized form" for an entire transaction/block and just store pointers to the start of various datastructures to provide accessors. Of course that could come later, but this is awesome by itself.

for reference, I found this unmaintained implementation that stores blockchain data structures in slices while providing walker to access structured data

Kixunil · 2021-10-12T13:11:24Z

Newtype for witness structure - yes please! It always felt strange to me having just Vec<Vec<u8>> with no newtype.

Storing block in serialized form with pointers - this may help us with electrs performance. We're troubleshooting/brainstorming right now: romanz/electrs#547 cc @romanz

I think it'd require a bit of reasonable unsafe (design similar to the bytes crate) or annoying lifetimes preventing caching.
But first let's see how urgent this is.

Kixunil

Concept ACK. Didn't do as deep review as I do usually. Maybe I will look at it more later.

src/blockdata/witness.rs

Kixunil · 2021-10-12T13:31:17Z

src/blockdata/witness.rs

+
+#[derive(Clone, PartialEq, Eq, PartialOrd, Ord, Debug, Hash)]
+#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]  //TODO manually implement?
+pub struct Witness {


Maybe have offsets in SmallVec?
We could also optimistically assume that transactions are smaller than 4M bytes, so we could cram offsets into struct U24(u8, u8, u8);

Kixunil · 2021-10-12T13:36:25Z

src/blockdata/witness.rs

+/// Support structure to allow efficient and convenient iteration over the Witness elements
+pub struct WitnessIterator<'a> {
+    witness: &'a Witness,
+    cursor: usize,


Why not just core::slice::Iter<'a, u8>?

Because you can't get slices from it and should not work as Read?

Or maybe I don't get what you have in mind

You can, it has as_slice() method. I used it in #662 and #673

Was also thinking of doing take_slice_or_kill via extension trait, so it could be reused. You seem to be doing something similar so maybe worth revisiting that idea.

(Was also considering adding it to core as it seems widely applicable.)

Added in 691179d

src/blockdata/witness.rs

Kixunil · 2021-10-12T13:40:49Z

src/blockdata/witness.rs

+    where
+        S: serde::Serializer,
+    {
+        let vec: Vec<_> = self.iter().map(|e| e.to_vec()).collect();


I think this could be faster but for initial code probably good. (Manual serde impls are annoying as hell.)

dpc · 2021-10-13T16:52:04Z

Storing block in serialized form with pointers - this may help us with electrs performance. We're troubleshooting/brainstorming right now: romanz/electrs#547 cc @romanz

I think it'd require a bit of reasonable unsafe (design similar to the bytes crate) or annoying lifetimes preventing caching.
But first let's see how urgent this is.

Not sure how applicable it is, but in rdedup I've used a patter of reading data into chunks of ArcRefs from https://docs.rs/owning_ref/0.4.1/owning_ref/ and then passing short lists of references to actual sub-pieces of data to achieve a zero-copy "garbage collected" handling of data-stream and references into it. I didn't have to use unsafe, though owning_ref must have some unsafe inside it.

Kixunil · 2021-10-13T17:37:17Z

@dpc yes, also bytes does something similar. I wanted to avoid doing a bunch of atomic operations for every transaction.

Kixunil

The latest update looks great!

Kixunil · 2021-10-14T09:47:06Z

src/blockdata/witness.rs

-            self.cursor = end;
-            Some(&vec[start..end])
+        let varint = VarInt::consensus_decode(self.0.as_slice()).ok()?;
+        self.0.nth(varint.len() - 1)?; // VarInt::len returns at least 1


I'd like to see VarInt::consensus_decode_iter() one day but good enough for this PR.

dr-orlovsky · 2021-11-12T19:34:44Z

Needs a rebase

dr-orlovsky

Without acessor methods to items inside Witness (Index impl or get) it is literally impossible to use this structure in many cases (like in miniscript etc)

It is also required to change type of the witness data in PSBT inputs.

src/blockdata/transaction.rs

dr-orlovsky · 2021-11-24T21:24:42Z

Can you also pls stick in Display implementation:

impl Display for Witness {
    fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
        for item in self.iter() {
            writeln!(f, "{}", item.to_hex())?;
        }
        Ok(())
    }
}

dr-orlovsky · 2021-11-24T22:13:58Z

Here are all the improvements I'd like to have in this PR: RCasatta#4

RCasatta · 2021-11-25T10:50:07Z

Without acessor methods to items inside Witness (Index impl or get) it is literally impossible to use this structure in many cases (like in miniscript etc)

I don't think it's literally impossible to access elements without Index, the options are:

calling nth on a new iterator (which is basically what the suggested RCasatta@bf3127d is doing)
collect the iterator and use the Vec<Vec<u8>>

Also, I can't find extensive indexed access in the miniscript lib (please point me to the code if I am wrong).

So I am not convinced to implement Index on this Witness type which could bring to inefficient code like

for i in 0..witness_len {
   do something with witness[i]
}

I am open to changing my mind, what do other people think?

Kixunil · 2021-11-25T18:38:14Z

Why Display? Sounds strange to display it. Debug definitely yes.

dpc · 2021-11-26T03:59:42Z

I am open to changing my mind, what do other people think?

This PR seems to be skipping deserializing witness, so not everyone has to pay for it, and code that do need it, can pay for it how they want. Seems like a win in most cases, and a small inconvenience for other.

dr-orlovsky · 2021-11-27T09:48:12Z

Why Display? Sounds strange to display it.

Well, I'd like to have in command-line pro wallets (I have one myself) printing tx witness like

deafbeef549805...
badcaffe544985...
...

And not

Witness {
	data: 
		135,
		76,
....

dr-orlovsky · 2021-11-27T09:51:16Z

@RCasatta replied in that PR, but briefly, we need things like "accessing second to last element" in many cases, including parsing witness data for taproot and segwit v0, so why not to have a index map as a part of the structure and const-time Index impls?

RCasatta · 2021-11-27T14:31:28Z

@RCasatta replied in that PR, but briefly, we need things like "accessing second to last element" in many cases, including parsing witness data for taproot and segwit v0, so why not to have a index map as a part of the structure and const-time Index impls?

Adding an index map would defeat the main [1] purpose of this PR [2] because it would require another allocation for the map.

If "accessing second to last element" is a common use case (and also the last, I guess), we can add two Option<NonZeroUsize> pointing to the last two elements and two efficient last() and second_to_last() methods

[1]: main because I think deserialization happens more than operating on the witness
[2]: speeding up deserialization

dr-orlovsky · 2021-11-27T14:43:04Z

I think that is a good solution!

dpc · 2021-11-28T02:07:05Z

If there are multiple strong usecases here, how about having two (or even multiple) Witness representation. There's some type system parametrization pollution involved, but everyone can get what they desire. Any type implementing consensus decode and encode could be used as witness (obviously it makes sense only for some actual types).

This new Witness becomes CompactWitness,
The Vec<Vec<u8>> becomes DeepWitness
if you have any other ideas, you can create AnotherWitness
TxIn becomes TxIn<W = CompactWitness>, and recursively any type holding witness gets a <W = CompactWitness> added. It is a bit of noise all over the implementation, but in a typical use one can still use TxIn and it defaults to CompactWitness, or one can type MyBlock = Block<DeepWitness> and so on and have a convenient aliases for non-default versions.

Edit. One could even have NullWitness that throws the whole thing away, for cases where it is not useful. Possibly this technique could be used in more places.

Edit. I guess some types don't need to implement both decode and encode. If any is missing it will just make them un-encodeable/decodeabe. A NullWitness is a good example of going one way only.

apoelstra · 2021-12-23T15:15:09Z

src/blockdata/witness.rs

+    cursor: usize,
+}
+
+impl From<Vec<Vec<u8>>> for Witness {


In c342adb:

I think the From trait is inappropriate here because you're doing a new allocation and the conversion is nontrivial. Can you pull this into an independent from_vec or something method? (Also, could you add a to_vec method which is ust a convenience wrapper over iterate-and-collect?)

Relatedly, in your serde Deserialize impl, I'm sure we can do something more efficient than decoding a Vec<Vec<u8>> and then converting -- though for this PR that's fine, we can do optimizations later.

I added 8d23404
If the commit is what you expected I guess I'll squash the last 4 commits in the first

Not going to squash since it received the ACK it looks it's fine like it is

apoelstra · 2021-12-23T15:19:19Z

Done reviewing 00b2dd5. Looks great to me, just one comment.

Thanks for adding the last and second-to-last accessors. I think these are pretty commonly useful, since in a complete witness stack the last element is the witness script, while the second-to-last is the actual "bottom" of the stack.

apoelstra

ACK 8d23404

sanket1729

Did a detailed code review. Looks good, left one comment about clear function.

I was going to ask for a pop function, but I think for verifying and interpreting scripts witnesses, we should convert it to Vec<Vec>.

sanket1729 · 2021-12-27T20:13:06Z

src/blockdata/witness.rs

+        self.witness_elements = 0;
+    }
+
+    /// Push a new element on the witness, require an allocation


nit: requires an allocation.

sanket1729 · 2021-12-27T20:19:04Z

src/blockdata/witness.rs

+        assert_eq!(witness_serialized, serialize(&witness));
+
+        //assert_eq!(32, std::mem::size_of::<Witness>());
+        //assert_eq!(24, std::mem::size_of::<Option<Vec<u8>>>());


nit: remove comments here?

They were removed in latest commit, but present in old one, after the squash they are removed completely

sanket1729 · 2021-12-27T20:24:28Z

src/blockdata/witness.rs

@@ -27,23 +28,35 @@ pub struct Witness {
    /// Number of elements in the witness.
    /// It is stored separately (instead of as VarInt in the initial part of content) so that method
    /// like [`Witness::push`] doesn't have case requiring to shift the entire array
-    witness_elements: u64,
+    witness_elements: usize,


last and second_to_last are not reset when calling the clear function

…eeping most usability. Witness struct is in place of the Vec<Vec<u8>> we have before this commit. from_vec() and to_vec() methods are provided to switch between this type and Vec<Vec<u8>> Moreover, implementation of Default, Iterator and others allows to have similar behaviour but using a single Vec prevent many allocations during deserialization which in turns results in better performance, even 20% better perfomance on recent block. last() and second_to_last() allows to access respective element without going through costly Vec transformation

RCasatta · 2021-12-28T09:15:18Z

Fixed the clear method.

I squashed all but the fuzz commit into one since they were changing the same code.

sanket1729

ACK 2fd0125

apoelstra

ACK 106acdc

RCasatta · 2021-12-28T17:53:41Z

ACK 2fd0125

Do you mean the tip commit?

RCasatta · 2021-12-29T16:55:15Z

@dr-orlovsky could you remove the "changes requested" if everything is ok? I cannot merge otherwise

sanket1729

ACK 106acdc

dr-orlovsky

@RCasatta sorry for making it hanging

utACK 106acdc

I have several notes, but I do not think it worth re-reviewing this PR due to them - they may be addressed in a follow-up PR.

dr-orlovsky · 2021-12-29T23:36:16Z

src/util/sighash.rs

@@ -657,10 +656,10 @@ impl<R: DerefMut<Target = Transaction>> SigHashCache<R> {
    ///     let prevout_script = Script::new();
    ///     let _sighash = sig_hasher.segwit_signature_hash(inp, &prevout_script, 42, EcdsaSigHashType::All);
    ///     // ... sign the sighash
-    ///     sig_hasher.witness_mut(inp).unwrap().push(Vec::new());
+    ///     sig_hasher.witness_mut(inp).unwrap().push(&Vec::new());


Nit:

Suggested change

/// sig_hasher.witness_mut(inp).unwrap().push(&Vec::new());

/// sig_hasher.witness_mut(inp).unwrap().push(&[]);

dr-orlovsky · 2021-12-29T23:37:38Z

src/blockdata/witness.rs

+#[cfg(feature = "serde")]
+use serde;


Nit: this seems redundant (we always have access to the crate without useing it)

dr-orlovsky · 2021-12-29T23:45:51Z

src/blockdata/witness.rs

+
+impl Witness {
+    /// Creates [`Witness`] object from an array of byte-arrays
+    pub fn from_vec(vec: Vec<Vec<u8>>) -> Self {


Nit: why not impl From<Vec<Vec<u8>> for Witness instead?

And probably pub fn from<T: AsRef<[u8]>>(bytes: &[T]) will be better.

Because From is for cheap/trivial conversions. This is neither.

From is for cheap/trivial conversions

Not really, impl From<&str> for String exists for instance and that conversion is not usually considered "cheap".

@dr-orlovsky I consider AsRef<[u8]> a footgun since it would accept strings without explicit as_bytes()

dr-orlovsky · 2021-12-29T23:47:01Z

src/blockdata/witness.rs

+    }
+
+    /// Returns a struct implementing [`Iterator`]
+    pub fn iter(&self) -> Iter {


I think the name here is confusing: it is not clear do we iterate over witness stack elements - or witness byte sequence. So I will prefer fn byte_iter(&self)

We iterate over witness stack elements. It wouldn't be meaningful to iterate over bytes. I don't think there is any ambiguity here.

From what I see in the code we do iterate over bytes here: self.content.iter() where content: Vec<u8> will iterate over bytes.

self.content is used internally to provide the witness stack elements iterator via iter()

I agree with @dr-orlovsky here, chars() and bytes() exist on &str for this reason.

It is clearly meaningful to iterate over either the characters or bytes of a string. It is not clear to me what it means to iterate over the bytes of a witness, or even in what order they would be returned.

OK, if it seems completely obvious that it means stack elements fine for me. I'd just like the docs to be more descriptive.

Just note that non-obvious iterators can be a footgun. I once got burned by &std::path::Path implementing IntoIterator (Can you guess what it iterates over without looking it up?)

…arking 247a14f Use test big block for bench_stream_reader instead of making one (Riccardo Casatta) b92dfbb exclude test_data when publishing the crate (Riccardo Casatta) f5a9681 include a big block in test_data, use it for ser/de benchmark (Riccardo Casatta) 09dada5 Move bip158 test vectors to test_data (Riccardo Casatta) 06d1a82 Remove testnet block hex from tests, use test_data with include_bytes! (Riccardo Casatta) Pull request description: In the first two commits I moved some data from source files to the newly introduced `test_data` dir, including it with `include_[str|bytes]!` macro. The second-to-last commit introduces a big block in test_data which is very handy in ser/de benchmark (I used it for #672) because with smaller blocks you may not notice performance improvements. Since I don't want to pollute the package the last commit excludes the `test_data` dir from the published package. I think it's fine to do it because dependent packages don't run dependencies tests. ACKs for top commit: apoelstra: ACK 247a14f Kixunil: tACK 247a14f Tree-SHA512: a2beb635b0a358737d0b57d3e7205b1ddf87652b9a8c889ce63e2867659a8eaf7e43a5b87a453345d56d953745913f40b58596f449e5fbc87340e0dd2aef0727

see rust-bitcoin/rust-bitcoin#672

…ex), and get_tapscript 3c0d5ae Add get_tapscript to Witness (junderw) 4226d60 Add Index<usize> and nth(index) to Witness (junderw) Pull request description: Ref: #672 (comment) [Add Index<usize> and nth(index) to Witness](4226d60) [4226d60](4226d60) Arbitrary indexing into Witness fixes the API of last and second_to_last to be more flexible. This patch started off as an addition of third_to_last, but ended up evolving into arbitrary indexing to allow for future use cases. A list of the indices of the start byte for each witness element is stored as an ordered contiguous group of u32s represented as 4 bytes each in the Vec<u8> contents. The bytes are stored using to_ne_bytes for performance reasons. A helper function is added to the tests to allow for easier contruction of the contents Vec in test vectors. u32 was chosen because 22 bits are needed to store 4,000,000 which is the maximum weight limit for a block. This might need to be reworked in the event of consensus limits increasing, but u32 can hold 1000x the current limit, so it should be fine for the forseeable future. The push and consensus_deserialize functions utilize rotate_left and rotate_right to move the indices to the end of the new allocation. Depending on the size of the data, this might be more of a performance hit than just allocating a new temporary Vec to store the indices and append them after parsing is completed. However, for a majority of cases rotating the indices should be faster. Suggestions to use VecDeque instead of Vec for contents necessitate other considerations, since it is not a public facing change, those optimizations can be dealt with in future patches. The Index<usize> trait is implemented using the new nth method with expect. The Iter struct is reworked to make use of the new data representation. This new data structure makes it trivial to implement DoubleEndedIterator and other such traits, but I have decided to leave this as out of scope for this patch. --- [Add get_tapscript to Witness](a7501d9) [a7501d9](a7501d9) This new method will check the last witness element to see if it starts with 0x50, and depending on the result it will return the second to last or third to last witness element according to BIP341. In its current state, Witness can not know what type of script it is fulfilling, so it is up to the caller to verify if the previous output is a taproot output or not. --- Edit: This is the previous PR body: > In a taproot script payment with annex, quick access to the 3rd to last element (which is the actual script in this case) is convenient. > > This feels like kicking the can down the road again, but I think it's a nice to have method. > > RCasatta dr-orlovsky were discussing this issue. I would like to ask if they have any thoughts on the addition of this. ACKs for top commit: tcharding: ACK 3c0d5ae apoelstra: ACK 3c0d5ae Kixunil: ACK 3c0d5ae Tree-SHA512: 0038eed6ad56786b8dd6d98db0d1846753b8b25de0bc1089cdc75d5850d0ccc66dde9a10be7fe09589ad7db118fd50ee9f7993695968df5c389457ccfcdaa761

RCasatta commented Sep 30, 2021

View reviewed changes

src/util/bip143.rs Outdated Show resolved Hide resolved

RCasatta force-pushed the witness_with_block branch from af9711e to 8dcf3d6 Compare October 1, 2021 10:59

TheBlueMatt reviewed Oct 1, 2021

View reviewed changes

RCasatta force-pushed the witness_with_block branch from 8dcf3d6 to 8ccc278 Compare October 5, 2021 13:40

RCasatta marked this pull request as ready for review October 5, 2021 13:49

Kixunil reviewed Oct 12, 2021

View reviewed changes

RCasatta mentioned this pull request Oct 13, 2021

Bench using Tinyvec in some places #679

Draft

Kixunil reviewed Oct 14, 2021

View reviewed changes

dr-orlovsky suggested changes Nov 23, 2021

View reviewed changes

src/blockdata/transaction.rs Outdated Show resolved Hide resolved

RCasatta mentioned this pull request Nov 25, 2021

Improvements to Witness type RCasatta/rust-bitcoin#4

Closed

apoelstra reviewed Dec 23, 2021

View reviewed changes

RCasatta force-pushed the witness_with_block branch 2 times, most recently from 231d784 to 8d23404 Compare December 23, 2021 16:52

apoelstra previously approved these changes Dec 24, 2021

View reviewed changes

RCasatta added the one ack PRs that have one ACK, so one more can progress them label Dec 27, 2021

sanket1729 reviewed Dec 27, 2021

View reviewed changes

RCasatta added 2 commits December 28, 2021 09:56

Add fuzzing for Witness struct

106acdc

RCasatta dismissed apoelstra’s stale review via ba224cf December 28, 2021 09:11

RCasatta force-pushed the witness_with_block branch from ba224cf to 106acdc Compare December 28, 2021 09:12

RCasatta removed the one ack PRs that have one ACK, so one more can progress them label Dec 28, 2021

sanket1729 approved these changes Dec 28, 2021

View reviewed changes

apoelstra approved these changes Dec 28, 2021

View reviewed changes

sanket1729 approved these changes Dec 29, 2021

View reviewed changes

dr-orlovsky approved these changes Dec 29, 2021

View reviewed changes

dr-orlovsky merged commit 86055d9 into rust-bitcoin:master Dec 29, 2021

RCasatta mentioned this pull request Dec 31, 2021

Use test_data for big objects, add big block for benchmarking #750

Merged

dr-orlovsky mentioned this pull request Jan 15, 2022

Bump version to v0.28 #785

Closed

RCasatta mentioned this pull request Jan 27, 2022

On performance and memory usage RCasatta/blocks_iterator#1

Open

sanket1729 mentioned this pull request Jun 9, 2022

Make Witness::element_at public #1047

Closed

sander2 added a commit to sander2/polkabtc-clients that referenced this pull request Oct 6, 2022

chore: migration: Vec<Vec<u8>> -> Witness

950f9b7

see rust-bitcoin/rust-bitcoin#672

sander2 added a commit to sander2/polkabtc-clients that referenced this pull request Oct 6, 2022

chore: migration: Vec<Vec<u8>> -> Witness

3be2eca

see rust-bitcoin/rust-bitcoin#672

sander2 added a commit to sander2/polkabtc-clients that referenced this pull request Oct 7, 2022

chore: migration: Vec<Vec<u8>> -> Witness

bbbd525

see rust-bitcoin/rust-bitcoin#672

junderw mentioned this pull request Oct 18, 2022

[blockdata::Witness] New Features: Index<usize>, nth(index), and get_tapscript #1323

Merged

	/// sig_hasher.witness_mut(inp).unwrap().push(&Vec::new());
	/// sig_hasher.witness_mut(inp).unwrap().push(&[]);

New Witness struct to improve ser/de perfomance #672

New Witness struct to improve ser/de perfomance #672

Conversation

RCasatta commented Sep 30, 2021 • edited Loading

dr-orlovsky commented Sep 30, 2021

RCasatta commented Oct 1, 2021

dr-orlovsky commented Oct 1, 2021

TheBlueMatt 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RCasatta Oct 13, 2021 • edited Loading

Choose a reason for hiding this comment

dpc commented Oct 1, 2021

RCasatta commented Oct 5, 2021

RCasatta commented Oct 12, 2021

Kixunil commented Oct 12, 2021

Kixunil left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RCasatta Oct 13, 2021 • 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

dpc commented Oct 13, 2021

Kixunil commented Oct 13, 2021

Kixunil left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dr-orlovsky commented Nov 12, 2021

dr-orlovsky left a comment • edited Loading

Choose a reason for hiding this comment

dr-orlovsky commented Nov 24, 2021

dr-orlovsky commented Nov 24, 2021

RCasatta commented Nov 25, 2021

Kixunil commented Nov 25, 2021

dpc commented Nov 26, 2021

dr-orlovsky commented Nov 27, 2021 • edited Loading

dr-orlovsky commented Nov 27, 2021

RCasatta commented Nov 27, 2021 • edited Loading

dr-orlovsky commented Nov 27, 2021

dpc commented Nov 28, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

apoelstra commented Dec 23, 2021

apoelstra left a comment

Choose a reason for hiding this comment

sanket1729 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

Choose a reason for hiding this comment

RCasatta commented Dec 28, 2021

sanket1729 left a comment

Choose a reason for hiding this comment

apoelstra left a comment

Choose a reason for hiding this comment

RCasatta commented Dec 28, 2021

RCasatta commented Dec 29, 2021

sanket1729 left a comment

Choose a reason for hiding this comment

dr-orlovsky 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

RCasatta commented Sep 30, 2021 •

edited

Loading

RCasatta Oct 13, 2021 •

edited

Loading

RCasatta Oct 13, 2021 •

edited

Loading

dr-orlovsky left a comment •

edited

Loading

dr-orlovsky commented Nov 27, 2021 •

edited

Loading

RCasatta commented Nov 27, 2021 •

edited

Loading

dpc commented Nov 28, 2021 •

edited

Loading

dr-orlovsky left a comment •

edited

Loading