Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: use new multiformats CID interface and exports #7

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

rvagg
Copy link
Member

@rvagg rvagg commented Jun 6, 2022

Ref: multiformats/js-multiformats#161

Won't pass without that PR being merged but intended as a demonstration of using the new API.

Without these changes, but using that multiformats branch, it compiles and the types are stable. This PR switches to using the CID interface but the concrete create, parse and asCID methods that are exported as statics off the 'multiformats/cid' export without touching the CID class directly at all.

I'm interested in the implications of the strict BitcoinBlockCID, BitcoinTxCID and BitcoinWitnessCommitmentCID are for users of this API - perhaps you just want to ingest CIDs and you don't care what types they are, do you now have to use a CID<number, number, CIDVersion> type rather than a CID to receive these?

@rvagg rvagg requested a review from Gozala June 6, 2022 07:09
@rvagg rvagg self-assigned this Jun 6, 2022
@rvagg
Copy link
Member Author

rvagg commented Jun 6, 2022

types diff for this is here:

types/
diff -Naur /tmp/btc-master-types/bitcoin-block.d.ts types/bitcoin-block.d.ts
--- /tmp/btc-master-types/bitcoin-block.d.ts	2022-06-06 16:24:44.167487051 +1000
+++ types/bitcoin-block.d.ts	2022-06-06 17:04:29.113672466 +1000
@@ -2,9 +2,8 @@
  * @template T
  * @typedef {import('multiformats/codecs/interface').ByteView<T>} ByteView
 */
-/**
- * @typedef {import('./interface').BitcoinHeader} BitcoinHeader
- */
+/** @typedef {import('./interface').BitcoinHeader} BitcoinHeader */
+/** @typedef {import('./interface').BitcoinBlockCID} BitcoinBlockCID */
 /**
  * **`bitcoin-block` / `0xb0` codec**: Encodes an IPLD node representing a
  * Bitcoin header object into byte form.
@@ -29,10 +28,10 @@
  * The process of converting to a CID involves reversing the hash (to little-endian form), encoding as a `dbl-sha2-256` multihash and encoding as a `bitcoin-block` multicodec. This process is reversable, see {@link cidToHash}.
  *
  * @param {string} blockHash a string form of a block hash
- * @returns {CID} a CID object representing this block identifier.
+ * @returns {BitcoinBlockCID} a CID object representing this block identifier.
  * @name BitcoinBlock.blockHashToCID()
  */
-export function blockHashToCID(blockHash: string): CID;
+export function blockHashToCID(blockHash: string): BitcoinBlockCID;
 /**
  * **`bitcoin-block` / `0xb0` codec**: the codec name
  * @name BitcoinBlock.name
@@ -45,5 +44,5 @@
 export const code: 176;
 export type ByteView<T> = import('multiformats/codecs/interface').ByteView<T>;
 export type BitcoinHeader = import('./interface').BitcoinHeader;
-import { CID } from "multiformats";
+export type BitcoinBlockCID = import('./interface').BitcoinBlockCID;
 //# sourceMappingURL=bitcoin-block.d.ts.map
\ No newline at end of file
diff -Naur /tmp/btc-master-types/bitcoin-tx.d.ts types/bitcoin-tx.d.ts
--- /tmp/btc-master-types/bitcoin-tx.d.ts	2022-06-06 16:24:44.167487051 +1000
+++ types/bitcoin-tx.d.ts	2022-06-06 17:04:29.113672466 +1000
@@ -33,10 +33,10 @@
  *
  * @name BitcoinTransaction.encodeAll()
  * @param {BlockPorcelain} obj
- * @returns {IterableIterator<{cid:CID, bytes:Uint8Array, transaction?:BitcoinBlockTransaction}>}
+ * @returns {IterableIterator<{cid:BitcoinTxCID, bytes:Uint8Array, transaction?:BitcoinBlockTransaction}>}
  */
 export function encodeAll(obj: BlockPorcelain): IterableIterator<{
-    cid: CID;
+    cid: BitcoinTxCID;
     bytes: Uint8Array;
     transaction?: BitcoinBlockTransaction;
 }>;
@@ -47,10 +47,10 @@
  *
  * @name BitcoinTransaction.encodeAllNoWitness()
  * @param {BlockPorcelain} obj
- * @returns {IterableIterator<{cid:CID, bytes:Uint8Array, transaction?:BitcoinBlockTransaction}>}
+ * @returns {IterableIterator<{cid:BitcoinTxCID, bytes:Uint8Array, transaction?:BitcoinBlockTransaction}>}
  */
 export function encodeAllNoWitness(obj: BlockPorcelain): IterableIterator<{
-    cid: CID;
+    cid: BitcoinTxCID;
     bytes: Uint8Array;
     transaction?: BitcoinBlockTransaction;
 }>;
@@ -74,10 +74,10 @@
  * The process of converting to a CID involves reversing the hash (to little-endian form), encoding as a `dbl-sha2-256` multihash and encoding as a `bitcoin-tx` multicodec. This process is reversable, see {@link cidToHash}.
  *
  * @param {string} txHash a string form of a transaction hash
- * @returns {CID} A CID (`multiformats.CID`) object representing this transaction identifier.
+ * @returns {BitcoinTxCID} A CID (`multiformats.CID`) object representing this transaction identifier.
  * @name BitcoinTransaction.txHashToCID()
  */
-export function txHashToCID(txHash: string): CID;
+export function txHashToCID(txHash: string): BitcoinTxCID;
 /**
  * **`bitcoin-tx` / `0xb1` codec**: the codec name
  * @name BitcoinTransaction.name
@@ -93,7 +93,7 @@
 export type BlockPorcelain = import('bitcoin-block/interface').BlockPorcelain;
 export type BitcoinTransaction = import('./interface').BitcoinTransaction;
 export type BitcoinTransactionMerkleNode = import('./interface').BitcoinTransactionMerkleNode;
-import { CID } from "multiformats";
+export type BitcoinTxCID = import('./interface').BitcoinTxCID;
 import { bytes } from "multiformats";
 import { BitcoinTransaction as BitcoinBlockTransaction } from "bitcoin-block";
 //# sourceMappingURL=bitcoin-tx.d.ts.map
\ No newline at end of file
diff -Naur /tmp/btc-master-types/bitcoin-witness-commitment.d.ts types/bitcoin-witness-commitment.d.ts
--- /tmp/btc-master-types/bitcoin-witness-commitment.d.ts	2022-06-06 16:24:44.167487051 +1000
+++ types/bitcoin-witness-commitment.d.ts	2022-06-06 17:04:29.113672466 +1000
@@ -1,11 +1,11 @@
 /**
  * @param {import('bitcoin-block/classes/Block').BlockPorcelain} deserialized
- * @param {CID|null} witnessMerkleRoot
- * @returns {{cid:CID, bytes:Uint8Array}|null}
+ * @param {BitcoinTxCID|null} witnessMerkleRoot
+ * @returns {{cid:BitcoinWitnessCommitmentCID, bytes:Uint8Array}|null}
  * @ignore
  */
-export function encodeWitnessCommitment(deserialized: import('bitcoin-block/classes/Block').BlockPorcelain, witnessMerkleRoot: CID | null): {
-    cid: CID;
+export function encodeWitnessCommitment(deserialized: import('bitcoin-block/classes/Block').BlockPorcelain, witnessMerkleRoot: BitcoinTxCID | null): {
+    cid: BitcoinWitnessCommitmentCID;
     bytes: Uint8Array;
 } | null;
 /**
@@ -49,5 +49,6 @@
 export type ByteView<T> = import('multiformats/codecs/interface').ByteView<T>;
 export type BlockPorcelain = import('bitcoin-block/classes/Block').BlockPorcelain;
 export type BitcoinWitnessCommitment = import('./interface').BitcoinWitnessCommitment;
-import { CID } from "multiformats";
+export type BitcoinTxCID = import('./interface').BitcoinTxCID;
+export type BitcoinWitnessCommitmentCID = import('./interface').BitcoinWitnessCommitmentCID;
 //# sourceMappingURL=bitcoin-witness-commitment.d.ts.map
\ No newline at end of file
diff -Naur /tmp/btc-master-types/complete.d.ts types/complete.d.ts
--- /tmp/btc-master-types/complete.d.ts	2022-06-06 16:24:44.167487051 +1000
+++ types/complete.d.ts	2022-06-06 17:04:29.113672466 +1000
@@ -8,10 +8,10 @@
  *
  * @name Bitcoin.encodeAll()
  * @param {BlockPorcelain} block
- * @returns {IterableIterator<{cid: CID, bytes: Uint8Array}>}
+ * @returns {IterableIterator<{cid: BitcoinBlockCID|BitcoinTxCID|BitcoinWitnessCommitmentCID, bytes: Uint8Array}>}
  */
 export function encodeAll(block: BlockPorcelain): IterableIterator<{
-    cid: CID;
+    cid: BitcoinBlockCID | BitcoinTxCID | BitcoinWitnessCommitmentCID;
     bytes: Uint8Array;
 }>;
 /**
@@ -26,11 +26,11 @@
  * `bitcoin-tx` and `bitcoin-witness-commitment` CIDs.
  *
  * @param {IPLDLoader} loader an IPLD block loader function that takes a CID argument and returns a `Uint8Array` containing the binary block data for that CID
- * @param {CID} blockCid a CID of type `bitcoin-block` pointing to the Bitcoin block header for the block to be assembled
+ * @param {BitcoinBlockCID} blockCid a CID of type `bitcoin-block` pointing to the Bitcoin block header for the block to be assembled
  * @returns {Promise<{deserialized:BlockPorcelain, bytes:Uint8Array}>} an object containing two properties, `deserialized` and `bytes` where `deserialized` contains a full JavaScript instantiation of the Bitcoin block graph and `bytes` contains a `Uint8Array` with the binary representation of the graph.
  * @name Bitcoin.assemble()
  */
-export function assemble(loader: IPLDLoader, blockCid: CID): Promise<{
+export function assemble(loader: IPLDLoader, blockCid: BitcoinBlockCID): Promise<{
     deserialized: BlockPorcelain;
     bytes: Uint8Array;
 }>;
@@ -39,6 +39,8 @@
 export type IPLDLoader = import('./interface').IPLDLoader;
 export type BitcoinTransaction = import('./interface').BitcoinTransaction;
 export type BitcoinTransactionMerkleNode = import('./interface').BitcoinTransactionMerkleNode;
-import { CID } from "multiformats";
+export type BitcoinBlockCID = import('./interface').BitcoinBlockCID;
+export type BitcoinTxCID = import('./interface').BitcoinTxCID;
+export type BitcoinWitnessCommitmentCID = import('./interface').BitcoinWitnessCommitmentCID;
 import { bytes } from "multiformats";
 //# sourceMappingURL=complete.d.ts.map
\ No newline at end of file
diff -Naur /tmp/btc-master-types/interface.d.ts types/interface.d.ts
--- /tmp/btc-master-types/interface.d.ts	2022-06-06 16:24:44.167487051 +1000
+++ types/interface.d.ts	2022-06-06 17:04:29.113672466 +1000
@@ -1,28 +1,38 @@
 import { TransactionInCoinbasePorcelain, TransactionInPorcelain } from 'bitcoin-block/classes/TransactionIn';
 import { BlockHeaderPorcelain, TransactionPorcelain } from 'bitcoin-block/interface';
-import { CID } from 'multiformats';
+import { CID } from 'multiformats/interface';
+export declare type HASH_ALG_CODE = 0x56;
+export declare type CODEC_BLOCK_CODE = 0xb0;
+export declare type CODEC_TX_CODE = 0xb1;
+export declare type CODEC_WITNESS_COMMITMENT_CODE = 0xb2;
 export declare type IPLDLoader = (cid: CID) => Promise<Uint8Array>;
+export interface BitcoinBlockCID extends CID<CODEC_BLOCK_CODE, HASH_ALG_CODE, 1> {
+}
+export interface BitcoinTxCID extends CID<CODEC_TX_CODE, HASH_ALG_CODE, 1> {
+}
+export interface BitcoinWitnessCommitmentCID extends CID<CODEC_WITNESS_COMMITMENT_CODE, HASH_ALG_CODE, 1> {
+}
 export interface BitcoinHeader extends BlockHeaderPorcelain {
-    parent: CID | null;
-    tx: CID;
+    parent: BitcoinBlockCID | null;
+    tx: BitcoinTxCID;
 }
 export interface BitcoinTransactionMerkleNode {
-    0: CID | null;
-    1: CID;
+    0: BitcoinTxCID | null;
+    1: BitcoinTxCID;
 }
 export interface BitcoinTransactionInCoinbase extends TransactionInCoinbasePorcelain {
-    tx: CID;
+    tx: BitcoinTxCID;
     txinwitness: [string];
 }
 export interface BitcoinTransactionIn extends TransactionInPorcelain {
-    tx: CID;
+    tx: BitcoinTxCID;
 }
 export interface BitcoinTransaction extends TransactionPorcelain {
-    witnessCommitment?: CID;
+    witnessCommitment?: BitcoinWitnessCommitmentCID;
     vin: (BitcoinTransactionInCoinbase | BitcoinTransactionIn)[];
 }
 export interface BitcoinWitnessCommitment {
-    witnessMerkleRoot: CID | null;
+    witnessMerkleRoot: BitcoinTxCID | null;
     nonce: Uint8Array;
 }
 //# sourceMappingURL=interface.d.ts.map
\ No newline at end of file
diff -Naur /tmp/btc-master-types/util.d.ts types/util.d.ts
--- /tmp/btc-master-types/util.d.ts	2022-06-06 16:24:44.167487051 +1000
+++ types/util.d.ts	2022-06-06 17:04:29.113672466 +1000
@@ -1,3 +1,6 @@
+/**
+ * @typedef {import('multiformats/interface').CID} CID
+ */
 /** @typedef {import('bitcoin-block/interface').BlockPorcelain} BlockPorcelain */
 /**
  * Instantiate a full object form from a full Bitcoin block graph binary representation. This binary form is typically extracted from a Bitcoin network node, such as with the Bitcoin Core `bitcoin-cli` `getblock <identifier> 0` command (which outputs hexadecimal form and therefore needs to be decoded prior to handing to this function). This full binary form can also be obtained from the utility {@link assemble} function which can construct the full graph form of a Bitcoin block from the full IPLD block graph.
@@ -46,8 +49,8 @@
 export function cidToHash(cid: CID | string): string;
 export const blockHashToCID: typeof block.blockHashToCID;
 export const txHashToCID: typeof tx.txHashToCID;
+export type CID = import('multiformats/interface').CID;
 export type BlockPorcelain = import('bitcoin-block/interface').BlockPorcelain;
-import { CID } from "multiformats/cid";
 import * as block from "./bitcoin-block.js";
 import * as tx from "./bitcoin-tx.js";
 //# sourceMappingURL=util.d.ts.map
\ No newline at end of file

Mostly it's just replacements of CID with the specific parameterised Bitcoin*CID types.

The most interesting piece I'm noticing with this diff, which I think might help answer my question above, is that the IPLDLoader type of (cid: CID) => Promise<Uint8Array> which is used as a way for a user to supply blocks given an arbitrary CID, gets to remain the same, with a plain CID, while it's called with the parameterised specific CID types. Similar for the cidToHash() in the utils module which remains generic CID.

However, I'm not running the tsc on the test/ directory here yet, so I'm not getting full calls into the API tested, which limits the amount of useful information I think.

@@ -1,5 +1,5 @@
import { BitcoinTransaction } from 'bitcoin-block'
import { CID } from 'multiformats'
import { create as createCID, asCID } from 'multiformats/cid'
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Diff may have end even smaller if you did

Suggested change
import { create as createCID, asCID } from 'multiformats/cid'
import * as CID from 'multiformats/cid'

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair, although there's so much hanging off CID now that I feel uncomfortable using it (a mental nit more than anything), including CID.CID.

But if you were to make an example usage of the new CID API, would you advise * as CID or the specific pieces? Let's make this an exemplar that we can point to until we have more & better docs.

@Gozala
Copy link

Gozala commented Jun 6, 2022

I'm interested in the implications of the strict BitcoinBlockCID, BitcoinTxCID and BitcoinWitnessCommitmentCID are for users of this API - perhaps you just want to ingest CIDs and you don't care what types they are, do you now have to use a CID<number, number, CIDVersion> type rather than a CID to receive these?

It depends. If you just ingest it should not matter and you could annotate as CID as interface defines those default parameters. However I do say it depends because:

  1. Ingestion more often implies passing those CIDs into other functions and if those do care about those details you don't get to ignore that. Usually it leads to repeating same type constraints as the functions you'd be calling internally.

  2. If passed CID ends up in output position (perhaps as part of a larger object) ignoring those type parameters may not be ideal, that is because CID coming out would loose type information and may become incompatible with functions that care about those details. Usually you'd want to use generics in such instances.

Hope this helps

@BigLep BigLep marked this pull request as draft July 19, 2022 22:08
@BigLep
Copy link

BigLep commented Jul 19, 2022

In draft because waiting on multiformats/js-multiformats#161

@BigLep
Copy link

BigLep commented Sep 6, 2022

We'll pick this up when there is a v10 pre-release.

v10 is happening here: multiformats/js-multiformats#199

@rvagg
Copy link
Member Author

rvagg commented Sep 7, 2022

Updated to use multiformats@10-pre and the Link interface, including making the custom Bitcoin*CID interfaces extensions of Link and making the loader: type IPLDLoader = (cid:Link)=>Promise<Uint8Array> which is probably breaking for some cases but may be a good addition.

Unfortunately now js-bitcoin-block needs updating because it uses multiformats and is CJS so the web tests don't work here because it's pulling in the wrong version in the bundle.

@BigLep
Copy link

BigLep commented Nov 15, 2022

2022-11-15 conversation: if we don't do anything, things are "ok". We didn't do a breaking change.
The only person who uses is @rvagg :)
Low priority

@BigLep BigLep added the P3 Low: Not priority right now label Nov 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P3 Low: Not priority right now
Projects
Status: 🥞 Todo
Development

Successfully merging this pull request may close these issues.

None yet

3 participants