Skip to content

Introduce the pkg/bitcoin package#3384

Merged
pdyraga merged 12 commits intomainfrom
bitcoin-groundwork
Nov 1, 2022
Merged

Introduce the pkg/bitcoin package#3384
pdyraga merged 12 commits intomainfrom
bitcoin-groundwork

Conversation

@lukasz-zimnoch
Copy link
Copy Markdown
Member

Refs: #3349

Here we introduce the new pkg/bitcoin package that defines some basic Bitcoin types and interfaces required to work with the Bitcoin chain. This package is meant to reflect the part of the Bitcoin protocol to an extent required by the client applications. The pkg/bitcoin package is supposed to hold components specific to the Bitcoin domain and must remain free of application-specific items. Third-party helper libraries (like btcd) are allowed for use within this package though no external type should leak outside. Preserving those rules will allow us to achieve a clean package structure with proper dependencies. The exact types introduced by this pull request are:

  • bitcoin.ByteOrder: An enum that captures all the caveats related to the byte order used across the Bitcoin ecosystem
  • bitcoin.CompactSizeUint: A documentation type that holds the details about the CompactSize Unsigned Integer
  • bitcoin.Hash: A type that represents a 32-byte Bitcoin hash widely used as identifier in the Bitcoin protocol
  • bitcoin.Transaction & related: A type that represents a Bitcoin transaction and related types to reflect the internals
  • bitcoin.BlockHeader: A representation of the Bitcoin block header that will be used for SPV purposes
  • bitcoin.Chain: A minimal interface required to interact with the Bitcoin chain
  • electrum.Client: An example stub implementation of the bitcoin.Chain supposed to establish a package structure for Bitcoin interfaces implementations

Here we declare the `ByteOrder` enum that is meant to capture all the
details related to Bitcoin byte order and the `CompactSizeUint` type
that holds the details about the Bitcoin CompactSize Unsigned Integer.
This type is meant to represent a 32-byte Bitcoin hash widely used as
identifier in the Bitcoin protocol.
Here we define `bitcoin.Transaction` and all its related types that capture
all the details related with Bitcoin transactions.
Here we define the representation of the Bitcoin block header that will be
used for SPV purposes.
Here we declare a minimal interface required to interact with the Bitcoin
chain.
This type is supposed to establish the package structure for the Bitcoin
chain implementations using ElectrumX as example.
Comment thread pkg/bitcoin/bitcoin.go Outdated
Comment thread pkg/bitcoin/chain.go Outdated
Comment thread pkg/bitcoin/hash.go Outdated
Comment thread pkg/bitcoin/hash.go Outdated
Comment thread pkg/bitcoin/block.go Outdated
Comment thread pkg/bitcoin/hash.go Outdated
Comment thread pkg/bitcoin/transaction.go Outdated
tomaszslabon
tomaszslabon previously approved these changes Oct 27, 2022
@tomaszslabon
Copy link
Copy Markdown
Contributor

Looks good to me now.
I leave it to @pdyraga and @nkuba.

Comment thread pkg/bitcoin/bitcoin.go

// CompactSizeUint is a documentation type that is supposed to capture the
// details of the Bitcoin's CompactSize Unsigned Integer as described in:
// https://developer.bitcoin.org/reference/transactions.html#compactsize-unsigned-integers
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Could we extract some information from the linked docs and put it here? For example, we could copy the table clarifying the types.

                    Value               | Bytes | Format
---------------------------------------------------------------------------------------
>= 0 && <= 252                          |   1   | uint8
>= 253 && <= 0xffff                     |   3   | 0xfd followed by the number as uint16
>= 0x10000 && <= 0xffffffff             |   5   | 0xfe followed by the number as uint32
>= 0x100000000 && <= 0xffffffffffffffff |   9   | 0xff followed by the number as uint64

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

See 2af23fe

Comment thread pkg/bitcoin/bitcoin.go Outdated
// use cases related with the protocol logic and cryptography. Byte arrays
// using this byte order should be converted to numbers according to
// the little-endian sequence.
InternalByteOrder = iota
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Shouldn't this enum reference the ByteOrder type?

Suggested change
InternalByteOrder = iota
InternalByteOrder ByteOrder = iota

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Indeed! Good catch! See d31a350

Comment thread pkg/bitcoin/bitcoin.go Outdated
// byte order that is typically used by the third party services like
// block explorers or Bitcoin chain clients. Byte arrays using this byte
// order should be converted to numbers according to the big-endian
// sequence.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

In Bitcoin docs, this type is named RPC Byte Order. Can we mention that in the comment?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

See 2af23fe

Comment thread pkg/bitcoin/block.go Outdated
// Bits determines the target threshold this block's header hash must be
// less than or equal to.
Bits uint32
// Nonce is the nonce used to generate the block.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
// Nonce is the nonce used to generate the block.
// Nonce is an arbitrary number miners change to modify the header hash
// in order to produce a hash less than or equal to the target threshold.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

See 2af23fe

Comment thread pkg/bitcoin/block.go Outdated
// MerkleRootHash is a hash derived from the hashes of all transactions
// included in this block.
MerkleRootHash Hash
// Time is a Unix epoch time when the block was created.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
// Time is a Unix epoch time when the block was created.
// Time is a Unix epoch time when the miner started hashing the header.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

See 2af23fe

Comment thread pkg/bitcoin/chain.go
// GetBlockHeader gets the block header for the given block number. If the
// block with the given number was not found on the chain, this function
// returns an error.
GetBlockHeader(blockNumber uint) (*BlockHeader, error)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Wouldn't it be more accurate to accept block hash as the input parameter here?
https://github.com/blockstream/esplora/blob/master/API.md#get-blockhashheader

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Based on experiences gathered during the development of the tBTC v2 Typescript library, getting the block by number will have more use cases than getting it by hash (not sure if we have at least one case of the latter at the moment). If we need to fetch by hash, we will just add another method to the bitcoin.Chain interface.

@pdyraga pdyraga merged commit 7ee8811 into main Nov 1, 2022
@pdyraga pdyraga deleted the bitcoin-groundwork branch November 1, 2022 15:23
@nkuba nkuba mentioned this pull request Nov 2, 2022
3 tasks
Comment thread pkg/bitcoin/chain.go
Comment on lines +23 to +25
// GetCurrentBlockNumber gets the number of the current block. If the
// current block was not determined, this function returns an error.
GetCurrentBlockNumber() (uint, error)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@lukasz-zimnoch should it return the latest mined (confirmed) block or the block that is currently being mined?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

To be more precise maybe we could name this function GetLatestBlockHeight?

tomaszslabon added a commit that referenced this pull request Dec 9, 2022
This PR implements Bitcoin package integration with [Electrum
API](https://electrumx-spesmilo.readthedocs.io/en/latest/protocol.html).

There are a couple of servers implementations supporting the Electrum
API:
- [ElectrumX](https://github.com/spesmilo/electrumx)
- [Esplora - Electrs by
Blockstream](https://github.com/Blockstream/electrs)
- [Fulcrum](https://github.com/cculianu/Fulcrum)

A list of publicly exposed Electrum protocol servers for Bitcoin can be
found here:
- mainnet: https://1209k.com/bitcoin-eye/ele.php?chain=btc
- testnet: https://1209k.com/bitcoin-eye/ele.php?chain=tbtc

We added integration tests to verify the implementation works correctly
with the most popular server implementations. To run the integration
tests execute:
```
go test -v -tags=integration ./...
```

There are a couple of TODOs in the code regarding the improvements that
will be handled in follow-up PRs.

Refs #3384
Refs #3369
Refs #3395
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants