BIP329: add optional data fields, fix a JSON type #1750
Conversation
|
cc: @craigraw |
|
ACK If there was appetite would go even further with added WAC optional field and attempt to cover as much common accounting needs. |
|
Below are my thoughts on specific fields and suggestions for improving the proposal while keeping it practical: Block Height and Timestamps Including the block height is very useful, especially for understanding transaction confirmation status. The timestamp, while useful in some cases, feels less critical. It can be derived if needed, and excluding it could simplify the format. Transaction and Fees Fees are a tricky field to standardize, especially for batched or multi-party ownership transactions. Fair Market Value and Rate The FMV field is probably the attribute I wanted the most. The problem with exchange rate is, that there are many of them and I would stick to one (your own) value. Keypath Okay. Value (in sats) While value (in sats) can be looked up using tools like Electrum, having it as part of the format is still a helpful convenience. Heights for Addresses Nah. Address reuse is a bad practice. While it’s tempting to include some additional fields (mainly confirmation block height, MFV and value in sats) for richer metadata, I believe the BIP should stick to the smallest common denominator for simplicity and broad compatibility. Generally, I'm okay to store and parse metadata from the text label field and delegating more advanced data aggregation, management, and lookups to external tools and services, like labelbase.space. That said, if @craigraw is open to merging changes into BIP-329, I am happy to adapt the python-bip329 library to support these extensions and make them accessible on the Labelbase platform as well. |
jonatack
added
Proposed BIP modification
Pending acceptance
Co-authored-by: Jon Atack <[email protected]>
Co-authored-by: Jon Atack <[email protected]>
Co-authored-by: Jon Atack <[email protected]>
Co-authored-by: Jon Atack <[email protected]>
Co-authored-by: Jon Atack <[email protected]>
Co-authored-by: Jon Atack <[email protected]>
Co-authored-by: Jon Atack <[email protected]>
Co-authored-by: Jon Atack <[email protected]>
Thanks, all accepted and merged. |
|
Latest push e63bfa4 indeed took all my feedback per @xavierfiechter any specific changes/removals you would strongly request? Pinging BIP author @craigraw for feedback. |
jonatack
changed the title
|
@jonatack I've already shared my points here, on Twitter, and in previous discussions. I also believe that having nothing left to remove has its own value and justification, just as incorporating (some) of the requested changes does. IMHO, ensuring a vendor-agnostic standard that is widely adopted by both the industry and community is more important to me than the exact specifications. I think we should prioritize this aspect while still considering the details. |
There was a problem hiding this comment.
Regarding the origin field, the line "The origin property should only appear where type is tx, and the spendable property only where type is output" should be amended to just "The spendable property should only appear where type is output" since origin can now be used on all types.
|
|
||
| === Transactions === | ||
|
|
||
| * <tt>height</tt>: An integer giving the block height where this fully confirmed transaction can be found. For transactions that are confirmed by less than 6 blocks, omit this field or provide a value of zero. |
There was a problem hiding this comment.
Is 6 block confirmation requirement really necessary? It makes the addition of this field more complicated, and seems overly defensive. A wallet label export is a "point-in-time" export since a label can change at any time. If the export is viewed as an event stream, when a reorg occurs a new event with the same txid can be sent. There may be a need I haven't understood here though.
|
|
||
| * <tt>height</tt>: An integer giving the block height where this fully confirmed transaction can be found. For transactions that are confirmed by less than 6 blocks, omit this field or provide a value of zero. | ||
|
|
||
| * <tt>time</tt>: ISO-8601 formatted timestamp of the block given by the "height" field, preferably in UTC, although ISO-8601 can represent local times. Example: <tt>2025-01-23T11:40:35-05:00</tt>. |
There was a problem hiding this comment.
The ISO-8601 defined time zone should be required if this field is present. Given the preference for UTC, a better example would be 2025-01-23T11:40:35Z.
|
|
||
| * <tt>time</tt>: ISO-8601 formatted timestamp of the block given by the "height" field, preferably in UTC, although ISO-8601 can represent local times. Example: <tt>2025-01-23T11:40:35-05:00</tt>. | ||
|
|
||
| * <tt>fee</tt>: Integer giving the number of Satoshis that went to the miner for this transaction. |
There was a problem hiding this comment.
No change needed here, but just noting this is generally only known for outgoing transactions, where all the inputs are from the wallet.
|
|
||
| * <tt>fee</tt>: Integer giving the number of Satoshis that went to the miner for this transaction. | ||
|
|
||
| * <tt>value</tt>: Signed integer giving the number of Satoshis that came into the wallet by this transaction. Will be negative when sats leave the wallet. Could be zero if it is a consolidation transaction that moves from old UTXO to new. |
There was a problem hiding this comment.
Referring to the second sentence - in the case of a consolidation transaction this will reflect the transaction fee, which is unlikely to be zero, so this should be amended to be clearer.
|
|
||
| * <tt>value</tt>: Signed integer giving the number of Satoshis that came into the wallet by this transaction. Will be negative when sats leave the wallet. Could be zero if it is a consolidation transaction that moves from old UTXO to new. | ||
|
|
||
| * <tt>rate</tt>: Exchange rate at time of transaction. This is the fiat value of a Bitcoin at the time of the transaction, based on user preferences for data source. Example: <tt>"rate": { "USD": 105620.00 }</tt> |
There was a problem hiding this comment.
I don't think this needs to be restricted to fiat currencies, but can be "the equivalent value of a Bitcoin in another currency at the time of the transaction". It should be noted in the BIP that multiple currencies can be specified in the mapping, and ISO 4217 currency codes should be preferred as keys where possible.
|
|
||
| * <tt>fmv</tt>: Fair market value of the input/output relative to some other currency, typically fiat. The value should be a mapping, from currency code to decimal number. Example: <tt>"fmv": { "USD": 1233.45 }</tt>. Most situations will have only a single currency value, and it represents the real price of the goods/services expressed in some fiat currency. This is not an exchange *rate*, but an absolute value. By dividing by the <tt>value</tt> (above), it is possible to calculate an effective change rate for the transaction. | ||
|
|
||
| * <tt>height</tt> and <tt>time</tt>: Same definition as defined in Transactions. To save space, if the time value is already shared in a transaction, only the height is required. |
There was a problem hiding this comment.
Regarding the second sentence: neither field is required, so this is confusing. Besides, if you are expecting the receiver to do a lookup based on received transactions for the time field, why not the height as well? I think it would be better to omit this sentence altogether, since BIP329 can be used in an event stream in which case previous events should not be relied upon.
|
|
||
| === Address === | ||
|
|
||
| * <tt>heights</tt>: a list of block heights that contain inputs that deposit to the address. |
There was a problem hiding this comment.
Should be amended to "that contain outputs that deposit to the address". Should also include the block heights of transactions with inputs that spend from the address. It should be noted that it can be an empty array to indicate the address is unused.
| @@ -131,14 +131,63 @@ The following fragment represents a wallet label export: | |||
| { "type": "addr", "ref": "bc1q34aq5drpuwy3wgl9lhup9892qp6svr8ldzyy7c", "label": "Address" } | |||
| { "type": "pubkey", "ref": "0283409659355b6d1cc3c32decd5d561abaac86c37a353b52895a5e6c196d6f448", "label": "Public Key" } | |||
| { "type": "input", "ref": "f91d0a8a78462bc59398f2c5d7a84fcff491c26ba54c4833478b202796c8aafd:0", "label": "Input" } | |||
| { "type": "output", "ref": "f91d0a8a78462bc59398f2c5d7a84fcff491c26ba54c4833478b202796c8aafd:1", "label": "Output" , "spendable" : "false" } | |||
| { "type": "output", "ref": "f91d0a8a78462bc59398f2c5d7a84fcff491c26ba54c4833478b202796c8aafd:1", "label": "Output", "spendable": false } | |||
There was a problem hiding this comment.
@sethforprivacy Note the type correction here.
|
I'm generally ok with all the proposed fields, since they reference publicly available information, or at least information that should be publicly available. A note on implementation: Historical fiat exchange rate information is becoming more difficult for open source wallet clients to retrieve, with Coingecko and Coinbase now requiring paid subscriptions for historical rate lookups. Only mempool.space provides an open API for historical rates, and then only for the last couple of years in a handful of major currencies. However, this presents an interesting opportunity for server-based implementations, which can add the exchange rate fields by processing this format as an event stream, and may have less need for historical rate access. |
jonatack
added
the
PR Author action required
If the goal is solely to move labels between cooperating wallets, then previously defined values are the absolute minimum needed. However, wallet data exports can serve other purposes and many values associated with
addresses, transactions and outputs are already on-hand for the wallet generating the export, and yet would be hard or impossible for importing tools to reconstruct.
This PR proposes a bunch of optional fields that could be useful to users of this data. All are optional.