-
Notifications
You must be signed in to change notification settings - Fork 162
Add /txs/package endpoint to submit tx packages #119
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
Conversation
|
I highly appreciate the effort to add See for example mempool/electrs#105 which seems to use another endpoint ( |
|
Hey, I agree with your proposed endpoints in mempool/electrs#105 and will happily adapt mine to that :) I might even adopt your implementation :) |
|
based on the comments by @stevenroose to adopt mempool/electrs#105 we will go ahead and merge that cc @moneyball |
Sounds like we're aligned on an API then. Thanks for resolving this! :) |
e5aa76c to
a9a39b1
Compare
|
Hi all, I (finally, my apologies) adapted this PR to resemble the same REST endpoint and response as what @wiz added for mempool. @shesek is it ok if the internal mempool handling is done in a separate PR? It's seems to require a little bit of a non-trivial refactor and I don't really have the time. I would prefer that to not block having this endpoint added. Also rebased to resolve some conflict. |
a9a39b1 to
9a4175d
Compare
|
@shesek any update on this?? |
|
Any updates on this? |
|
Any update on this? |
|
merged via #159 |
…ync client cf64f97 feat(client): add new `submit_package` API (acidbunny21) Pull request description: ### Description Add new `submit_package` method to both `BlockingClient` and `AsyncClient`, as it's now supported by Esplora API, see: Blockstream/electrs#119, Blockstream/electrs#159. Also, adds new `SubmitPackageResult`, `TxResult`, and `MempoolFeesSubmitPackage` API structures. It updates the internal `post_request_hex` method to `post_request_bytes`, now handling `query_params` and having `Response` as return type. Additionally, updates the internals of `broadcast` to utilize the new `post_request` and `post_request_bytes` without breaking its API. ### Notes to the reviewers As mentioned in the PR comments, I updated the original commit by `@acidbunny21` by: - (i) adding `@ValuedMammal`'s suggested fixes; - (ii) removing the breaking change on the `broadcast` API, it's now a follow-up in #151. ### Changelog notice ``` ### Added - feat(client): add new `submit_package` API to `BlockingClient` and `AsyncClient` - feat(api): add new `SubmitPackageResult`, `TxResult`, and `MempoolFeesSubmitPackage` API structures ### Changed - feat(client): update the `post_request_hex` method to `post_request_bytes`, now handling `query_params` and having `Response` as return type. - feat(client): update the internals of the `broadcast` method to use new `post_request` and `post_request_bytes`, with no breaking change. ``` ### Checklists #### All Submissions: * [x] I've signed all my commits * [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md) * [x] I ran `just p` before pushing #### New Features: * [ ] I've added tests for the new feature > The PR does not add new tests for the `submit_package` API, as it's not yet supported by `electrsd`, it's tracked by: #152 * [x] I've added docs for the new feature ACKs for top commit: ValuedMammal: ACK cf64f97; used `git range-diff` luisschwab: re-ACK cf64f97 Tree-SHA512: cfc0a4ad3d119f52e503facbfcb3666c9fa521707d76016cb506bc4ad5261e0a9d1e9fdfcfcaa6de329b44535bc428c76d7dba47e437a0877a828786420ac1a3
I'm not sure if those stability tests etc are ran in CI. I'll let CI run and please let me know if manual testing is advised. It won't work with versions before 28.0-rc anyway.
Is there a common practice here for what to do when an api endpoint requires a certain daemon version? I guess now the node will throw an error and it will just be propagated upwards to the user.