Closed
Conversation
We will start with integrating with electrs.
The handle is used for integration with Electrs.
The function uses config to build the connection.
The wrappers to request get and post calls with retries.
GetTransaction returns a bitcoin transaction of the given transaction hash as TX ID. The function calls Electrs API to get the transaction and converts the returned transaction details to the format expected by the common Bitcoin handle.
We can simplify the code when we return bytes from response body instead of passing a reader.
The function returns the height of the last block. See: https://github.com/Blockstream/esplora/blob/master/API.md#get-blockstipheight
The function returns number of confirmations the transaction has. It gets the transaction details and extracts the status data. If the transaction is not found it retrurns an error. If the transaction is not confirmed it returns `0`. If the transaction is confirmed it compares the block number in which transaction was included with the current block tip. If the current block tip is the same number as transaction block number then it retruns `1`. If the current block tip is greater transaction block number it returns the difference incremented by 1. Examples: latest block height = 2000 transaction block height = 2000 result = 1 latest block height = 2020 transaction block height = 2000 result = 21
The function gets the hash of the block at specfic height and then gets the header of this block. The block returned in electrs/esplora format is converted to the expected bitcoin block type.
The function serializes the transaction and broadcast it.
If the status code is not 200 we use response body in the error message as a nice-to-have. There is no need to handle the error or log it, as this is a failure anyway.
The intention of the function is to return the most recent block that was mined (added to the chain) - also called a "tip". Replacing `Current` with `Latest` seems accurate and should reduce the confusion, as `current` may suggest the block that is currently being mined.
For Bitcoin it seems more common to use the name `block height` over the `block number`.
nkuba
commented
Nov 4, 2022
| GetCurrentBlockNumber() (uint, error) | ||
| // GetLatestBlockHeight gets the height of the latest block (tip). If the | ||
| // latest block was not determined, this function returns an error. | ||
| GetLatestBlockHeight() (uint, error) |
Member
Author
There was a problem hiding this comment.
@lukasz-zimnoch I hope you're fine with this rename.
The function expects the "chain tip", so the most recent mined block. I propose we use "latest", as "current" may be confusing and suggest that the block is being currently mined.
Also, I replaced "block number" with "block height", as it seems to be widely used in the Bitcoin ecosystem.
The basic set of unit tests for Electrs integration with mocked HTTP client.
For unit tests we need a value to be returned by the `Serialize` function. Until the function is implemented we will use a hardcoded value.
tomaszslabon
reviewed
Nov 4, 2022
| "github.com/keep-network/keep-core/pkg/bitcoin" | ||
| ) | ||
|
|
||
| type blockHeader struct { |
Contributor
There was a problem hiding this comment.
Let's not forget about descriptions of all the functions and types.
| Nonce: b.Nonce, | ||
| } | ||
|
|
||
| previousBlockHeaderHash, err := bitcoin.NewHashFromString(b.PreviousBlockHash, bitcoin.ReversedByteOrder) |
Contributor
There was a problem hiding this comment.
Could we split this lengthy line?
The integration test connects to the real Electrs API exposed by the Blockstream. To run the test execute: go test -v -tags=integration ./...
tomaszslabon
reviewed
Nov 7, 2022
| } | ||
|
|
||
| // Connect initializes handle with provided Config. | ||
| func Connect(config Config) (bitcoin.Chain, error) { |
Contributor
There was a problem hiding this comment.
Connect won't return an error even if the URL provided in the config is wrong.
We will only learn about the wrong URL when we try to use the Bitcoin chain for the first time.
I would consider adding a connection test before we leave Connect, so that we know we didn't make any mistake. We did such test when connecting to btc client in the old relay maintainer.
Member
Author
|
Closing in favor of #3406 |
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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR implements Bitcoin package integration with Electrs API.
TODO:
Refs #3384
Refs #3369