Update rust-bitcoin to v0.30.1 & rust-elements to v0.23.0 #59
Conversation
Some API JSON data types were updated to match changes in the underlying rust-bitcoin data types: - vsize and weights were changed from u32 to u64 (in `GET /tx/:txid`, `GET /block/:hash`, `GET /mempool` and `GET /mempool/recent`) - The `fee_histogram` was changed from [(f32,u32)] to [(f64,u64)] (in `GET /mempool` and Electrum's `mempool.get_fee_histogram`) - Block mining difficulty was changed from u64 to u128 (in GET `/block/:hash`) The actual JSON string serialization should remain unchanged, as serde-json serializes u64/u128/f64 as standard JSON numbers. However, numbers over ECMAScript's MAX_SAFE_INTEGER (2⁵³-1) may now produce JSON that some clients may fail to read correctly, while previously they would overflow (or error) on the server side. This shouldn't matter in practice because there shouldn't be such large numbers for weights, vsizes or mining difficulty. And if there are, the new behaviour seems (arguably?) better. To retain DB compatibility, block weights are converted to a u32 for the BlockMeta storage. This is OK because u32::MAX is far above MAX_BLOCK_WEIGHT.
Some REST API structs were changed to hold the underlying data type directly (instead of unnecessary manual ToHex conversion, which is no longer available in rust-bitcoin), but their JSON serialization should remain unchanged.
Use the underlying rust-bitcoin data types directly in more places to avoid unnecessary manual JSON string serialization. For cases where manual conversion is necessary, use the 'hex-conservative' crate (already dependant on by rust-bitcoin) instead of the 'hex' crate, which is now removed.
Made available in rust-bitcoin as of rust-bitcoin/rust-bitcoin#1707
This matches the behavior of bitcoin::LockTime, however
elements::LockTime uses serde-json's tagged enum serialization format,
which broke API compatibility.
This was detected by the check-api-stablity script:
Checking GET /tx/f89e3ec661950493e833beeba13bcd80e907a56c5f0e9ab47dc4f0810488a8e8 ...
@@ -2,3 +2,5 @@
"fee": 24910,
- "locktime": 73,
+ "locktime": {
+ "Blocks": 73
+ },
"size": 8942,
|
I added a simple script to ensure the changes didn't break HTTP API stablity. It detected one incompatibility that I fixed, and two more that are expected:
Edit: and one more, noticed by LeoComandini - |
|
utACK 95272ab, code review |
| }, | ||
| assetamount: match issuance.amount { | ||
| Value::Explicit(value) => Some(value), | ||
| Value::Null => Some(0), |
There was a problem hiding this comment.
This converts now to None, but I think it's more appropriate
There was a problem hiding this comment.
When is this Null? My check-api-stablity script didn't catch this because it appears to typically be either Explicit or Confidential
There was a problem hiding this comment.
tokenamount is Null in a issuance without reissuance token.
Adding
asset3=$(cli issueasset 10 0 false)
should show the breaking change
e.g. here it is 0, https://blockstream.info/liquid/api/tx/5221eb18850f88955214950c13fbff766bd1b33743099b6c05a504407585401a
There was a problem hiding this comment.
Thanks!
I added a test for this in 4a2d465. It now detects the incompatibility:
Checking GET /tx/f92f1f6638d4f3304ad28a0287206d552592f2678b719f2df9b552d4d021b2a3 ...
--- /dev/fd/63 2024-02-08 13:11:18.129749079 +0200
+++ /dev/fd/62 2024-02-08 13:11:18.129749079 +0200
@@ -18,4 +18,3 @@
"contract_hash": "0000000000000000000000000000000000000000000000000000000000000000",
- "is_reissuance": false,
- "tokenamount": 0
+ "is_reissuance": false
},
It's kind of weird to me that elements represents this as a null rather than an explicit 0, but I guess that it makes sense for us to follow suit.
Without this, this would sometimes report false positives due to non-deterministic array ordering in some of the endpoint (primarly mempool-related).
Of an issuance with no reissuance tokens, following Blockstream#59 (review). Previously this was reported as 0 and now as None (serializes to JSON without the field).
Feature: Install popular-scripts as a cronjob
2 participants
No description provided.