bitcoindevkit
feat(client): add async and blocking clients to submit txs package
Counterpart PR for https://github.com/Blockstream/electrs/pull/119 to enable submitpackage API
Pull Request Test Coverage Report for Build 19651633198
Details
- 33 of 107 (30.84%) changed or added relevant lines in 3 files are covered.
- 8 unchanged lines in 2 files lost coverage.
- Overall coverage decreased (-5.1%) to 82.238%
| Changes Missing Coverage | Covered Lines | Changed/Added Lines | % |
|---|---|---|---|
| src/api.rs | 0 | 12 | 0.0% |
| src/async.rs | 14 | 44 | 31.82% |
| src/blocking.rs | 19 | 51 | 37.25% |
| <!-- | Total: | 33 | 107 |
| Files with Coverage Reduction | New Missed Lines | % |
|---|---|---|
| src/async.rs | 1 | 72.24% |
| src/blocking.rs | 7 | 71.58% |
| <!-- | Total: | 8 |
| Totals | |
|---|---|
| Change from base Build 19620365463: | -5.1% |
| Covered Lines: | 1051 |
| Relevant Lines: | 1278 |
💛 - Coveralls
It could use a test with mempool/electrs implementation, while it's not available in blockstream/electrs and electrsd, as I've looked on the other PR and looks like it's following the same API.
I'm okay with adding a test, but then we need to bump electrsd to 0.30 which introduce some breaking changes on bitcoind. Might be good to first bump it in a separated PR
It could use a test with mempool/electrs implementation, while it's not available in blockstream/electrs and electrsd, as I've looked on the other PR and looks like it's following the same API.
I'm okay with adding a test, but then we need to bump electrsd to 0.30 which introduce some breaking changes on bitcoind. Might be good to first bump it in a separated PR
Yes, agree that it can be done in another PR. Another way that I thought was adding a test (under #[ignore]) that uses live mempool.space to test it, so we could run locally and in another step on CI while we don't have a new release on electrsd with the blockstream's esplora changes. However I think it's better to just wait until blockstream/electrs#119 is release and just bump everything.
@oleonardolima it looks like the pipeline failures are caused by something unrelated with the PR, could you help please? :)
@oleonardolima it looks like the pipeline failures are caused by something unrelated with the PR, could you help please? :)
It's caused due to recent native-tls release which bumped their MSRV to 1.80.0, it should be fine after #116 lands and a rebase should fix it.
I think we uncovered a bug in here:
Reqwest(reqwest::Error { kind: Decode, source: Error("invalid type: floating point `0.0`, expected u64", line: 1, column: 230) })
I suspect it's in this field: pub base: Amount,. You should never use Amount's default deserialization. It has several options, but bitcoind does amounts as floating points:
#[serde(with = "amount::serde::as_btc")]
pub base: Amount,
the above should work
It's weird, running locally CI works just fine on master. I've tried to re-run CI steps without success.
@tnull @ValuedMammal let me know if i can be of any help to have this merged
@acidbunny21 Is it okay if I take over this one? I want to move forward with this one and cut a release for rust-esplora-client, as I know @tnull has been looking forward to this.
@acidbunny21 Is it okay if I take over this one? I want to move forward with this one and cut a release for rust-esplora-client, as I know @tnull has been looking forward to this.
Yeah sure