Address to optimized QR string #581

RCasatta · 2021-03-22T13:04:29Z

Add a method to_qr_string for Address which is the same as to_string given by Display but it's uppercased if the address is bech32. Useful to embed the address in QR codes

dpc · 2021-03-22T17:12:48Z

Why is upper-case better for QR codes? You might want to put it into function documentation, for educational reasons.

RCasatta · 2021-03-23T08:46:14Z

@dpc, improved doc in cac3f46

dpc · 2021-03-23T19:35:52Z

Thanks! I've learned something. :)

Kixunil · 2021-03-23T20:41:31Z

You may also be interested in a large discussion at BTCPay see also the link in the issue itself.

sgeisler

Concept ACK. I think a slightly nicer way to implement this would be by using the alternative formatting mode. If you want the to_qr_string method could stay, but just call format!("{:#}", self). This way we can avoid unnecessary allocations (e.g. if someone wants to prefix the address with bitcoin:…).

Eventually (but outside of this PR's scope) it would be nice to allow upper case formatting directly in the bech32 crate.

RCasatta · 2021-03-31T14:50:53Z

Eventually (but outside of this PR's scope) it would be nice to allow upper case formatting directly in the bech32 crate.

isn't this necessary to avoid unnecessary allocations?

Kixunil · 2021-03-31T14:54:55Z

@RCasatta IDK exact details but I believe it should be possible to map the chars and turn them into upper cased.

dr-orlovsky

I'm up for having # alternative formatting in Display and using that for constructing the return in the to_qr_string method. Also, do not have a strong opinion but propose to consider to name the method to_string_uppercase (since if is more rusty and may be used outside of QR tasks where uppercase can be prefferred for typographic reasons etc).

RCasatta · 2021-04-06T13:14:49Z

Also, do not have a strong opinion but propose to consider to name the method to_string_uppercase

I don't think it's appropriate since pre-segwit addresses are not uppercased

sgeisler · 2021-04-06T14:28:48Z

isn't this necessary to avoid unnecessary allocations?

Even without the char mapping trick you save an allocation in case of non-bech32 addresses. But more importantly it allows to use the often preferable format API. And once the bech32 change lands downstream users won't need to change anything to use the more efficient version.

dr-orlovsky · 2021-04-06T18:34:04Z

I don't think it's appropriate since pre-segwit addresses are not uppercased

Ok, agree on this

RCasatta · 2021-04-07T13:56:12Z

Even without the char mapping trick you save an allocation in case of non-bech32 addresses.

Now allocates only for bech32 using alternate formatting

I don't know how to do the char mapping trick

Kixunil · 2021-04-07T14:05:18Z

I don't know how to do the char mapping trick

struct ToUpperWriter<W: fmt::Write>(W);

impl<W: fmt::Write> fmt::Write for ToUpperWriter {
    write_str(&mut self, s: &str) -> fmt::Result {
        for c in s.chars() {
            self.0.write_char(c.to_ascii_uppercase())?
        }
        Ok(())
    }
}

write!(ToUpperWriter(&mut fmt), "{}", self)

Kixunil · 2021-04-07T15:52:45Z

src/util/address.rs

+                    Network::Regtest => "bcrt",
+                };
+                let is_alternate = fmt.alternate();
+                let mut opt_up_writer = OptionallyUpperWriter(fmt, is_alternate);


I believe dispatching here instead of using bool is a bit faster, but probably not something to worry about.

Idk if that's what you mean (I think you mean putting the if here), but also why supply a boolean derived from fmt when you supply fmt?

but also why supply a boolean derived from fmt when you supply fmt?

the naive reason is that fmt is taken as something that impl fmt::Write, not as Formatter, so it's lacking alternate()

@sgeisler yes, I meant putting the if there as was written before.

not sure how to put the if as was before since something like this won't work

let bech32_writer = if fmt.alternate() { let upper = UpperWriter(fmt); bech32::Bech32Writer::new(hrp, &mut upper)?; } else { bech32::Bech32Writer::new(hrp, fmt)?; }

and other solutions I can think about are too verbose

Wait, wasn't your initial concern essentially avoiding a single function call in some cases? Always using dynamic dispatch sounds even worse (but maybe the compiler will figure it out? but the same cou). I still see that this is more elegant than having a boolen argument.

Also: we are horribly over-engineering this PR 😆

I only pointed out that evaluating if multiple times is not great without realizing it can't be fixed without adding bloat. Thus dynamic dispatch is probably preferable. Manually-implemented dynamic dispatch has no benefit over compiler-implemented dynamic dispatch and the latter has less boilerplate, thus should be preferred.

Yeah, we're overengineering, I personally enjoy it from time to time. 😄

In this example #581 (comment) there is also a lifetime issue not only a dispatching issue...

I am not sure how to go ahead:

"like it is now" -> use OptionallyUpperWriter with the bool

"verbose" -> use UpperWriter and if fmt.is_alternate() with the last two lines present in both of the if branch

"very verbose" -> use UpperWriter and OwnedWriter

"inner"

let mut upper_writer = UpperWriter(fmt); let writer = if upper_writer.0.alternate() { &mut upper_writer as &mut dyn fmt::Write } else { &mut upper_writer.0 as &mut dyn fmt::Write };

I miss what you are suggesting, please provide some code

I personally still prefer 1)

Ah, didn't realize the lifetime issue before. There's still a useful Rust trick to fix it, made a PR against your branch. :)

very nice! merged

src/util/address.rs

This replaces manually-written dynamic dispatch with `&mut dyn fmt::Write` which is hopefully more readable.

shesek · 2021-04-09T17:27:46Z

How about changing this to a to_uri method (or to_bip21_uri) instead, and also including the URI schema in it?

You probably want to include the URI schema in the QR anyway, and the URI could be useful for other non-QR uses too.

Also, to get a maximally efficient QR, the URI schema should be uppercased too, which this utility method could automatically take care of.

Kixunil · 2021-04-09T17:33:37Z

@shesek Maybe have a Bip21 (name to be bikesheded) struct (builder) that can also contain amount, description, payjoin link... Then it could follow the same pattern as here - alternate formatting using QR-optimized encoding and a to_qr_string convenience method.

Kixunil

I believe that apart from doc typo we're good now?

src/util/address.rs

sgeisler

tACK 3158ced

Test log:

Apr 13 18:15:16.914  INFO testinator: Installing rust toolchain 'nightly'
Apr 13 18:15:45.600  INFO testinator: Installing rust toolchain 'stable'
Apr 13 18:15:46.218  INFO testinator: Installing rust toolchain '1.29.0'
Apr 13 18:15:47.257  INFO testinator: Preparing environment for rust nightly tests (9 configurations)
Apr 13 18:15:47.257  INFO testinator: Preparing environment for rust stable tests (8 configurations)
Apr 13 18:15:47.257  INFO testinator: Preparing environment for rust 1.29.0 tests (8 configurations)
Apr 13 18:19:04.381  INFO testinator: Running rust nightly tests in /tmp/rust-bitcoin-nightly.jSKKUNtwODr9/rust-bitcoin
Apr 13 18:19:04.385  INFO testinator: Running rust 1.29.0 tests in /tmp/rust-bitcoin-1.29.0.sabYrOTO77Dz/rust-bitcoin
Apr 13 18:19:04.385 DEBUG testinator: Generating lock file with rust=1.29.0
Apr 13 18:19:04.387  INFO testinator: Running rust stable tests in /tmp/rust-bitcoin-stable.vU3c1dRZbz7L/rust-bitcoin
Apr 13 18:19:12.967 DEBUG testinator: Pinning cc to 1.0.41
Apr 13 18:19:13.118 DEBUG testinator: Pinning serde to 1.0.98
Apr 13 18:19:13.250 DEBUG testinator: Pinning serde_derive to 1.0.98
Apr 13 18:19:13.392 DEBUG testinator: Pinning byteorder to 1.3.4
Apr 13 18:20:00.288  INFO testinator: Test rust=nightly, features=[secp-recovery] succeeded!
Apr 13 18:20:08.001  INFO testinator: Test rust=stable, features=[secp-recovery] succeeded!
Apr 13 18:20:42.896  INFO testinator: Test rust=1.29.0, features=[secp-recovery] succeeded!
Apr 13 18:20:53.504  INFO testinator: Test rust=nightly, features=[use-serde] succeeded!
Apr 13 18:21:14.510  INFO testinator: Test rust=stable, features=[use-serde] succeeded!
Apr 13 18:21:18.721  INFO testinator: Test rust=nightly, features=[base64] succeeded!
Apr 13 18:21:45.890  INFO testinator: Test rust=nightly, features=[secp-lowmemory] succeeded!
Apr 13 18:21:47.192  INFO testinator: Test rust=stable, features=[base64] succeeded!
Apr 13 18:22:09.734  INFO testinator: Test rust=nightly, features=[rand] succeeded!
Apr 13 18:22:24.927  INFO testinator: Test rust=stable, features=[secp-lowmemory] succeeded!
Apr 13 18:22:43.193  INFO testinator: Test rust=1.29.0, features=[use-serde] succeeded!
Apr 13 18:22:43.369  INFO testinator: Test rust=nightly, features=[unstable] succeeded!
Apr 13 18:22:55.192  INFO testinator: Test rust=stable, features=[rand] succeeded!
Apr 13 18:23:12.843  INFO testinator: Test rust=nightly, features=[bitcoinconsensus] succeeded!
Apr 13 18:23:38.209  INFO testinator: Test rust=stable, features=[bitcoinconsensus] succeeded!
Apr 13 18:23:43.684  INFO testinator: Test rust=nightly, features=[secp-recovery,use-serde,base64,secp-lowmemory,rand,unstable,bitcoinconsensus] succeeded!                                                                                                  
Apr 13 18:23:47.501  INFO testinator: Test rust=1.29.0, features=[base64] succeeded!
Apr 13 18:24:09.789  INFO testinator: Test rust=nightly, features=[] succeeded!
Apr 13 18:24:15.883  INFO testinator: Test rust=stable, features=[secp-recovery,use-serde,base64,secp-lowmemory,rand,bitcoinconsensus] succeeded!                                                                                                            
Apr 13 18:24:42.014  INFO testinator: Test rust=stable, features=[] succeeded!
Apr 13 18:24:49.194  INFO testinator: Test rust=1.29.0, features=[secp-lowmemory] succeeded!
Apr 13 18:25:33.460  INFO testinator: Test rust=1.29.0, features=[rand] succeeded!
Apr 13 18:26:22.480  INFO testinator: Test rust=1.29.0, features=[bitcoinconsensus] succeeded!
Apr 13 18:27:11.257  INFO testinator: Test rust=1.29.0, features=[secp-recovery,use-serde,base64,secp-lowmemory,rand,bitcoinconsensus] succeeded!                                                                                                            
Apr 13 18:27:56.263  INFO testinator: Test rust=1.29.0, features=[] succeeded!
Apr 13 18:27:58.541  INFO testinator: Fuzzing deserialize_script
Apr 13 18:29:16.176  INFO testinator: Successfully fuzzed deserialize_script
Apr 13 18:29:16.176  INFO testinator: Fuzzing uint128_fuzz
Apr 13 18:30:18.167  INFO testinator: Successfully fuzzed uint128_fuzz
Apr 13 18:30:18.167  INFO testinator: Fuzzing deserialize_amount
Apr 13 18:31:19.181  INFO testinator: Successfully fuzzed deserialize_amount
Apr 13 18:31:19.181  INFO testinator: Fuzzing deserialize_transaction
Apr 13 18:32:21.104  INFO testinator: Successfully fuzzed deserialize_transaction
Apr 13 18:32:21.104  INFO testinator: Fuzzing deser_net_msg
Apr 13 18:33:24.254  INFO testinator: Successfully fuzzed deser_net_msg
Apr 13 18:33:24.254  INFO testinator: Fuzzing deserialize_address
Apr 13 18:34:25.239  INFO testinator: Successfully fuzzed deserialize_address
Apr 13 18:34:25.239  INFO testinator: Fuzzing deserialize_block
Apr 13 18:35:27.180  INFO testinator: Successfully fuzzed deserialize_block
Apr 13 18:35:27.181  INFO testinator: Fuzzing outpoint_string
Apr 13 18:36:29.078  INFO testinator: Successfully fuzzed outpoint_string
Apr 13 18:36:29.078  INFO testinator: Fuzzing deserialize_psbt
Apr 13 18:37:32.089  INFO testinator: Successfully fuzzed deserialize_psbt

shesek · 2021-04-13T18:04:10Z

I think this should include the URI schema, it makes it possible to scan the QR using the phone's built in QR scanner and not only within the wallet app.

And as mentioned before, the URI schema could also be optimized using upper casing (but only when the address is bech32), which would be nice to have as part of the library. If one were to take the output of the current to_qr_string() implementation and prepend it with a lowercase bitcoin:, it would result in a mixed-encoding QR split to two parts, one optimized and one not, with an extra 20 bit flag to switch between the two modes.

RCasatta · 2021-04-13T18:27:12Z

Do you think only the address has no utility at all?

A separate PR with a Bip21 builder like suggested by @Kixunil taking an address in constructor, and methods to optionally set amount, message and label may be better than having a single method with 3 optional parameters.

shesek · 2021-04-13T19:04:23Z

I think that if its meant to be used as a QR, including the URI schema would be more correct and useful than just the address. Including just the address does have utility, but it basically means that the responsibility for adding the QR URI schema is pushed to the library consumer, which seems unnecessary and prone to mistakes.

Adding the URI schema to the QR string doesn't mean that it also needs to support all of BIP21's parameters, this could be done separately, or not at all.

sgeisler · 2021-04-13T20:49:53Z

but it basically means that the responsibility for adding the QR URI schema is pushed to the library consumer

I guess adding the schema here would be in line with our goal of not forcing unnecessary allocations on users. Even with a encode_bip21(&self) -> Bip21Builder function we'd probably like a shorthand for the trivial case without arguments. Once bip21 is implemented to_qr_string(&self) could be implemented as self.encode_bip21().to_string() and we should ensure that the output stays the same.

That being said, I don't have a strong opinion on it (I'm not necessarily a fan of such URIs, but I see that it is a standard we probably want to support as a library).

RCasatta · 2021-04-14T08:29:45Z

I had a chat with @shesek and now I think adding the schema to the method to_qr_string it's better than not having it.

if you want to copy/paste the content to search the address on a blockexplorer the URI may be inconvenient, however, I just noticed on blocstream.info you can search by URI
if you really want only the address you can use the alternate formatting, also the inverse is true but more cumbersome (because you have to optionally uppercase also the schema)
if you are wondering the : is in the alphanumeric mode in QRs
It's right from a library level to enforce a good practice
the future encode_bip21 may handle more corner cases and have a flag without_uri

So I am going to change the PR unless @Kixunil @sgeisler @dr-orlovsky are against and prefer like it is now

Kixunil · 2021-04-14T12:21:08Z

if it's less convenient to destroy privacy, it may be a good thing :)
This troubles me, it looks like inconsistency. In that case I propose that uppercasing and QR string are different features. Doc needs to be updated to not confuse developers that they are same.
didn't know, nice!
I agree
I really like what @sgeisler suggested - have convenience method returning Bip21Builder; FTR I'd still love to see this builder regardless of other decisions.

To avoid confusion I propose for this PR:

rename to_qr_string to to_qr_uri or to_qr_uri_string (former seems better to me)
Put the docs about alternate encoding on top of impl fmt::Display ..., explain that to_qr_uri is strongly recommended for QR codes (the allocation will be an issue in embedded only)

Kixunil · 2021-04-14T12:29:07Z

Giving it another thought maybe return struct Uri(String) which will impl Display and later we can add URI parsing?

sgeisler · 2021-04-14T15:09:53Z

Giving it another thought maybe return struct Uri(String) which will impl Display and later we can add URI parsing?

I think there is too much uncertainty about how such an URI type should look and which APIs it should support (which is correlated to internal representation), it's most likely out of scope here. Just returning a string seems the safest bet, that will always be useful as a shorthand.

dr-orlovsky · 2021-04-14T15:58:13Z

Agree with @sgeisler and agree with reformatting PR into returning URL string

dr-orlovsky

Not sure about the function naming (no opinion there), otherwise utACK

dr-orlovsky · 2021-04-15T09:17:55Z

src/util/address.rs

+    ///
+    /// Quoting BIP 173 "inside QR codes uppercase SHOULD be used, as those permit the use of
+    /// alphanumeric mode, which is 45% more compact than the normal byte mode."
+    pub fn to_qr_uri(&self) -> String {


Still not sure about the best naming, since the function returns URI string, not URI object

It's good enough imo, I'm no fan of unneccesarily duplicating the signature in the name and the returned string is a valid URI.

I was thinking about this too since if we later add URI, then This will suck.

However, in reality, if you call a method like this you probably want to display it or encode it (e.g. using serde), not inspect its contents in the same way you would after parsing. It may be not beautiful, but it's pragmatic.

sgeisler

tACK 0a91496

Test log:

Apr 15 12:04:04.211  INFO testinator: Installing rust toolchain 'nightly'
Apr 15 12:04:36.485  INFO testinator: Installing rust toolchain 'stable'
Apr 15 12:04:37.070  INFO testinator: Installing rust toolchain '1.29.0'
Apr 15 12:04:38.225  INFO testinator: Preparing environment for rust stable tests (8 configurations)
Apr 15 12:04:38.225  INFO testinator: Preparing environment for rust 1.29.0 tests (8 configurations)
Apr 15 12:04:38.225  INFO testinator: Preparing environment for rust nightly tests (9 configurations)
Apr 15 12:07:56.236  INFO testinator: Running rust nightly tests in /tmp/rust-bitcoin-nightly.FRJRXVEgT9Lk/rust-bitcoin
Apr 15 12:07:56.240  INFO testinator: Running rust 1.29.0 tests in /tmp/rust-bitcoin-1.29.0.XAzppzzOZSDr/rust-bitcoin
Apr 15 12:07:56.240 DEBUG testinator: Generating lock file with rust=1.29.0
Apr 15 12:07:56.243  INFO testinator: Running rust stable tests in /tmp/rust-bitcoin-stable.Uy9SFUjp6pfb/rust-bitcoin
Apr 15 12:08:00.942 DEBUG testinator: Pinning cc to 1.0.41
Apr 15 12:08:01.104 DEBUG testinator: Pinning serde to 1.0.98
Apr 15 12:08:01.798 DEBUG testinator: Pinning serde_derive to 1.0.98
Apr 15 12:08:01.991 DEBUG testinator: Pinning byteorder to 1.3.4
Apr 15 12:08:41.812  INFO testinator: Test rust=nightly, features=[secp-recovery] succeeded!
Apr 15 12:08:47.875  INFO testinator: Test rust=stable, features=[secp-recovery] succeeded!
Apr 15 12:09:31.604  INFO testinator: Test rust=1.29.0, features=[secp-recovery] succeeded!
Apr 15 12:09:36.110  INFO testinator: Test rust=nightly, features=[use-serde] succeeded!
Apr 15 12:09:45.996  INFO testinator: Test rust=stable, features=[use-serde] succeeded!
Apr 15 12:09:59.658  INFO testinator: Test rust=nightly, features=[base64] succeeded!
Apr 15 12:10:17.604  INFO testinator: Test rust=stable, features=[base64] succeeded!
Apr 15 12:10:25.622  INFO testinator: Test rust=nightly, features=[secp-lowmemory] succeeded!
Apr 15 12:10:51.530  INFO testinator: Test rust=nightly, features=[rand] succeeded!
Apr 15 12:10:53.992  INFO testinator: Test rust=stable, features=[secp-lowmemory] succeeded!
Apr 15 12:11:20.097  INFO testinator: Test rust=nightly, features=[unstable] succeeded!
Apr 15 12:11:25.031  INFO testinator: Test rust=stable, features=[rand] succeeded!
Apr 15 12:11:28.269  INFO testinator: Test rust=1.29.0, features=[use-serde] succeeded!
Apr 15 12:12:00.338  INFO testinator: Test rust=nightly, features=[bitcoinconsensus] succeeded!
Apr 15 12:12:05.144  INFO testinator: Test rust=stable, features=[bitcoinconsensus] succeeded!
Apr 15 12:12:33.023  INFO testinator: Test rust=1.29.0, features=[base64] succeeded!
Apr 15 12:12:36.359  INFO testinator: Test rust=nightly, features=[secp-recovery,use-serde,base64,secp-lowmemory,rand,unstable,bitcoinconsensus] succeeded!                                                                                                  
Apr 15 12:12:46.642  INFO testinator: Test rust=stable, features=[secp-recovery,use-serde,base64,secp-lowmemory,rand,bitcoinconsensus] succeeded!                                                                                                            
Apr 15 12:13:01.144  INFO testinator: Test rust=nightly, features=[] succeeded!
Apr 15 12:13:18.042  INFO testinator: Test rust=stable, features=[] succeeded!
Apr 15 12:13:32.581  INFO testinator: Test rust=1.29.0, features=[secp-lowmemory] succeeded!
Apr 15 12:14:15.659  INFO testinator: Test rust=1.29.0, features=[rand] succeeded!
Apr 15 12:15:05.146  INFO testinator: Test rust=1.29.0, features=[bitcoinconsensus] succeeded!
Apr 15 12:15:57.331  INFO testinator: Test rust=1.29.0, features=[secp-recovery,use-serde,base64,secp-lowmemory,rand,bitcoinconsensus] succeeded!                                                                                                            
Apr 15 12:16:42.838  INFO testinator: Test rust=1.29.0, features=[] succeeded!
Apr 15 12:16:45.054  INFO testinator: Fuzzing deserialize_script
Apr 15 12:18:03.161  INFO testinator: Successfully fuzzed deserialize_script
Apr 15 12:18:03.161  INFO testinator: Fuzzing uint128_fuzz
Apr 15 12:19:05.080  INFO testinator: Successfully fuzzed uint128_fuzz
Apr 15 12:19:05.080  INFO testinator: Fuzzing deserialize_amount
Apr 15 12:20:06.070  INFO testinator: Successfully fuzzed deserialize_amount
Apr 15 12:20:06.070  INFO testinator: Fuzzing deserialize_transaction
Apr 15 12:21:08.248  INFO testinator: Successfully fuzzed deserialize_transaction
Apr 15 12:21:08.248  INFO testinator: Fuzzing deser_net_msg
Apr 15 12:22:11.190  INFO testinator: Successfully fuzzed deser_net_msg
Apr 15 12:22:11.191  INFO testinator: Fuzzing deserialize_address
Apr 15 12:23:12.178  INFO testinator: Successfully fuzzed deserialize_address
Apr 15 12:23:12.178  INFO testinator: Fuzzing deserialize_block
Apr 15 12:24:14.141  INFO testinator: Successfully fuzzed deserialize_block
Apr 15 12:24:14.141  INFO testinator: Fuzzing outpoint_string
Apr 15 12:25:15.067  INFO testinator: Successfully fuzzed outpoint_string
Apr 15 12:25:15.067  INFO testinator: Fuzzing deserialize_psbt
Apr 15 12:26:18.065  INFO testinator: Successfully fuzzed deserialize_psbt

Kixunil · 2021-04-15T12:28:21Z

src/util/address.rs

+    ///
+    /// Quoting BIP 173 "inside QR codes uppercase SHOULD be used, as those permit the use of
+    /// alphanumeric mode, which is 45% more compact than the normal byte mode."
+    pub fn to_qr_uri(&self) -> String {


I was thinking about this too since if we later add URI, then This will suck.

However, in reality, if you call a method like this you probably want to display it or encode it (e.g. using serde), not inspect its contents in the same way you would after parsing. It may be not beautiful, but it's pragmatic.

Kixunil · 2021-04-15T12:30:54Z

src/util/address.rs

    }
 }

+// Alternate formatting `{:#}` is used to return uppercase version of bech32 addresses which should
+// be used in QR codes, see [Address::to_qr_uri]


I suggest informing the readers of best practices:

s/see [Address::to_qr_uri]/please use [Address::to_qr_uri] unless you really have to avoid alocation/

apoelstra

ack 0a91496

Kixunil · 2021-11-22T18:06:34Z

Address to optimized QR string #581

Address to optimized QR string #581

Conversation

RCasatta commented Mar 22, 2021

dpc commented Mar 22, 2021

RCasatta commented Mar 23, 2021

dpc commented Mar 23, 2021

Kixunil commented Mar 23, 2021

sgeisler left a comment

Choose a reason for hiding this comment

RCasatta commented Mar 31, 2021

Kixunil commented Mar 31, 2021

dr-orlovsky left a comment

Choose a reason for hiding this comment

RCasatta commented Apr 6, 2021 • edited

sgeisler commented Apr 6, 2021

dr-orlovsky commented Apr 6, 2021

RCasatta commented Apr 7, 2021

Kixunil commented Apr 7, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RCasatta Apr 8, 2021 • edited

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 Apr 9, 2021 • edited

Choose a reason for hiding this comment

shesek commented Apr 9, 2021

Kixunil commented Apr 9, 2021

Kixunil left a comment

Choose a reason for hiding this comment

sgeisler left a comment • edited

Choose a reason for hiding this comment

shesek commented Apr 13, 2021 • edited

RCasatta commented Apr 13, 2021 • edited

shesek commented Apr 13, 2021

sgeisler commented Apr 13, 2021

RCasatta commented Apr 14, 2021

Kixunil commented Apr 14, 2021

Kixunil commented Apr 14, 2021

sgeisler commented Apr 14, 2021

dr-orlovsky commented Apr 14, 2021

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

sgeisler left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

apoelstra left a comment

Choose a reason for hiding this comment

Kixunil commented Nov 22, 2021

RCasatta commented Apr 6, 2021 •

edited

RCasatta Apr 8, 2021 •

edited

RCasatta Apr 9, 2021 •

edited

sgeisler left a comment •

edited

shesek commented Apr 13, 2021 •

edited

RCasatta commented Apr 13, 2021 •

edited