w3c-ccg / didm-btcr

WORK ITEM: BTCR DID Method Spec
https://w3c-ccg.github.io/didm-btcr/
Other
5 stars 4 forks source link

BTCR uses tx references that deviate from Bech32 Encoded Transaction Position References #1

Closed kimdhamilton closed 5 years ago

kimdhamilton commented 6 years ago

Intro

Originally, BTCR used Bech32 Encoded Transaction Position References for transaction references, but over time they deviated. We should decide (1) if we really want to do this, and if so, (2) ensure we've properly documented our deviation.

Terminology

For the moment I'll use the following placeholder terminology to refer to our modified form:

Problem

BTCR truncates Bech32 Encoded Transaction Position References (txref) by removing the network prefix. So for example, a txref of txtest1-xkyt-fzgq-qq87-xnhn converts to a txref-ext of xkyt-fzgq-qq87-xnhn. The conversion library still uses txrefs, so that we can match the testcases referred to in the reference implementation. But the BTCR libraries handle this conversion by removing/adding back the txref prefix. See the btcr-did-tools library, see util.ensureTxref.

The primary reason was to shorten the DIDs, but we discussed at RWoT 5 whether this deviation was worth it.

The following additional reasons were mentioned:

Opening an issue to confirm this decision and follow up on items (1)-(3)

Reference:

danpape commented 6 years ago

Hi!

As for (1), yes, I believe we do want to do this. For (2), I'm not sure how to properly document... should we fork/edit the original BIP, or copy/augment and make a new BIP?

Another item to discuss, which you mentioned above as an "additional" reason, is adding the tx output index to the txref-ext. I've been extending the C++ library Ryan and I are developing, and I have added extra bits to the original txref definition. Here are a few examples:

txref (orig) ... implicit utxoIndex of 0 (block=0, txPos=0, uxtoIndex=0) => rqqq-qqqq-qmhu-qk (block=466793, txPos=2205, uxtoIndex=0) => rjk0-u5ng-4jsf-mc (block=466793, txPos=2205, uxtoIndex=10) => not possible

txref-ext (with explicit utxoIndex) (block=0, txPos=0, uxtoIndex=0) => rqqq-qqqq-qqqu-au7hl (block=466793, txPos=2205, uxtoIndex=0) => rjk0-u5ng-qqq8-lsnk3 (block=466793, txPos=2205, uxtoIndex=10) => rjk0-u5ng-2qqn-tcg6h

Note that I've deviated from adding a final "-" before the very last character in the txref-ext, since I thought that looked weird. We can discuss that.

ChristopherA commented 6 years ago

I suggest we in parallel try to get them to modify/update their BIP — I think the generic construction might be useful to Lightning.

Also, we need to create registry in CCG of Magic Codes for chains, as BIP dropped Litecoin. There can only be a limited number of codes, so we should reserve the least useful one for extension.

danpape commented 6 years ago

Pinging this issue to bring it back up... Tagging @rxgrant as well.

I haven't been keeping up very much with the original discussion for the txref BIP... does that seem like something that will happen someday, or has it been overtaken by something else? Ryan and I have been using the longer txref (with embedded txo index) for some time now. We haven't gotten to submitting a pull request for our diff from the BIP-xxxx document, but I hope we will soon.

Thoughts?

danpape commented 5 years ago

As I'm editing various RWOT documents, I thought I would comment in here. Since the last comment in August, BIP-0136 has been updated, though still not merged in to the master set of BIPs. I have updated our C++ reference library and also Kim's javascript reference library. I would not close this issue yet, as BIP-0136 has not yet been accepted. I'll post a followup comment when I have any updates.

kimdhamilton commented 5 years ago

Notice that I updated the btcr playground to be consistent with txref-conversion, but I didn’t strip off the chain-specific prefix. I just realized that when I saw Dan’s pr, which indicates we are still stripping that prefix off in the resulting did, deviating from the bip.

I’m feeling like our deviation isn’t worth the effort. The only reason was size reduction, right?

ChristopherA commented 5 years ago

I think the extra characters are a waste when it also implied by the x.

— Christopher Allen [via iPhone]

On Thu, Dec 13, 2018 at 7:27 PM Kim (Hamilton) Duffy < notifications@github.com> wrote:

Notice that I updated the btcr playground to be consistent with txref-conversion, but I didn’t strip off the chain-specific prefix. I just realized that when I saw Dan’s pr, which indicates we are still stripping that prefix off in the resulting did, deviating from the bip.

I’m feeling like our deviation isn’t worth the effort. The only reason was size reduction, right?

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/w3c-ccg/didm-btcr/issues/1#issuecomment-447201290, or mute the thread https://github.com/notifications/unsubscribe-auth/AAEN7165-QPDJKbYo-1YplW9ptDtDpayks5u4xqlgaJpZM4TF_jv .

kimdhamilton commented 5 years ago

I think there’s a cost for deviating from a standard, and I’m not sure it’s worth it in this case. I.e even if extra characters are redundant, is the reduced size really that much of a gain?

@rxgrant @danpape

Curious to hear from others

kimdhamilton commented 5 years ago

Related question: in the original comment we called out these items as reasons for deviating:

-In the future we may also want to encode a specific tx output -Ability to refer to alt chains -Ability to refer to lightning channel ids

If the BIP still doesn’t address these, and we will likely deviate anyway, I will relax my opposition

ChristopherA commented 5 years ago
danpape commented 5 years ago

In the latest version of BIP-136, which veleslavs has requested to be merged (https://github.com/veleslavs/bips/blob/txrev_v2/bip-0136.mediawiki) we state that

Please note: other specifications, such as the Decentralized Identifiers spec, have implicitly encoded the information contained within the HRP elsewhere. In this case they may choose to not include the HRP as specified here.

After introducing the four magic codes, he also states his intention that

Other magic codes will be specified in SLIP-XXXX "TxRef for Non-Bitcoin Chains and Networks".

... though I don't know if he has any specific plans about this yet.

To summarize the magic codes: In the current BIP revision (and the current implementations of our C++ and Kim's javascript libraries) the "extended" txrefs (with tx output) are identified using new magic codes: main=3, main extended=4, test=6, test extended=7. This results in the first character of every txref being 'r', 'x', 'y' and '8', respectively. Therefore, txrefs that include the bech32 "HRP" look like:

Of course, once we turn them into DIDs, they start to look pretty noisy:

did:btcr:txtest1:8qqq-qqqq-qyrq-pwl4-tv

I know I have heard people say "No one should ever have to see a DID", but I like how they look without the HRP:

did:btcr:8qqq-qqqq-qyrq-pwl4-tv

kimdhamilton commented 5 years ago

Thanks @danpape, this is helpful. It does simplify the DID strings, and I like that BIP-136 already mentions potential deviation in the HRP.

These reasons make sense to me, and I support this approach. I'll update the playground to reflect this.

kimdhamilton commented 5 years ago

Updated playground, but I'll keep this issue open until we document these decisions in the relevant places

kimdhamilton commented 5 years ago

the ability to drop the HRP is documented in BIP 136, and we address this in the method spec. Ok to close