Implement method to submit package of transactions (submitpackage call)#288
Implement method to submit package of transactions (submitpackage call)#288SomberNight merged 1 commit intospesmilo:masterfrom
Conversation
docs/protocol-methods.rst
Outdated
| The Bitcoin Core daemon result according to its RPC API documentation. | ||
|
|
||
| **Result Example** |
There was a problem hiding this comment.
Hmm. Not sure if we want to just forward the return value from bitcoind as-is.
The bitcoin core RPC docs say: This RPC is experimental and the interface may be unstable.
If we do want to just forward this result, I guess we too should mark the result as unstable (?).
There was a problem hiding this comment.
I agree that forwarding the result could cause problems in clients if the e-x server upgrades its daemon to a new version and the result structure changes, while defining our own structure would limit the issues to the e-x server side which seems less problematic.
There was a problem hiding this comment.
I added a verbose parameter so the user can decide if he wants the full response or only the package_msg and replaced txids. See PR spesmilo/electrum-protocol#1
157ca93 to
336a16a
Compare
d39138b to
50a161b
Compare
|
Simple script to test the call: import socket
def main():
with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as s:
s.connect(("127.0.0.1", 51001))
s.sendall(b'{"jsonrpc": "2.0", "method": "server.version", "params": ["", "1.4.4"], "id": 0}\n')
data = s.recv(1028)
s.sendall(b'{"jsonrpc": "2.0", "method": "blockchain.transaction.broadcast_package", "params": [["raw_parent_tx", "raw_child_tx"], false], "id": 0}\n')
data = s.recv(1028)
print(data)
if __name__ == '__main__':
main() |
|
Commit ff7c1a3 represents the protocol changes in commit spesmilo/electrum-protocol@7bc5f11 |
|
7de390f represents protocol pr status spesmilo/electrum-protocol@3e49ee8 |
SomberNight
left a comment
There was a problem hiding this comment.
Thanks! This looks good.
I added some comments but I will fix them myself in a follow-up.
| except ValueError: | ||
| self.logger.info(f"error calculating txids: {traceback.format_exc()}") |
There was a problem hiding this comment.
no need to import traceback
| except ValueError: | |
| self.logger.info(f"error calculating txids: {traceback.format_exc()}") | |
| except ValueError as e: | |
| self.logger.info(f"error calculating txids", exc_info=e) |
here is some boilerplate:
import traceback
import logging
import sys
logger = logging.getLogger(__name__)
logger.setLevel(logging.DEBUG)
console_stderr_handler = logging.StreamHandler(sys.stderr)
console_stderr_handler.setLevel(logging.DEBUG)
logger.addHandler(console_stderr_handler)and now compare (in a fresh local python interpreter):
try:
txids = [double_sha256(bytes.fromhex(tx)).hex() for tx in tx_package]
except Exception:
logger.info(f"error calculating txids: {traceback.format_exc()}")and
try:
txids = [double_sha256(bytes.fromhex(tx)).hex() for tx in tx_package]
except Exception as e:
logger.info(f"error calculating txids", exc_info=e)and
try:
txids = [double_sha256(bytes.fromhex(tx)).hex() for tx in tx_package]
except Exception:
logger.info(f"error calculating txids", exc_info=True)see https://docs.python.org/3/library/logging.html#logging.Logger.debug
There was a problem hiding this comment.
Thanks, that's useful to know!
| if ptuple >= (1, 4, 4): | ||
| handlers['blockchain.transaction.broadcast_package'] = self.package_broadcast |
There was a problem hiding this comment.
Let's either not expose the new method at all, or maybe expose it unconditionally by moving it ~6 lines higher into the big dict.
I don't want to gate it behind an arbitrary temporary version number. Once spesmilo/electrum-protocol#2 is done, we can gate it behind that version number along with the other changes.
Until it is documented in the spec, clients should not rely on this method existing at any version.
| raw_txs: a list of raw transactions as hexadecimal strings""" | ||
| self.bump_cost(0.25 + sum(len(tx) / 5000 for tx in tx_package)) | ||
| try: | ||
| txids = [double_sha256(bytes.fromhex(tx)).hex() for tx in tx_package] |
There was a problem hiding this comment.
What are these txids for? They are in reversed byte order versus normal RPC interface, so logging them isn't that helpful, because it's not like you can copy-paste them to an explorer to view the txns (unless you manually reverse them first).
Also, they are wtxids, and not normal txids, because you are hashing the raw txn which contains witness data, rather than the stripped txn...
There was a problem hiding this comment.
Thanks, good catch. Yeah this is incorrect i remove it in this followup PR #319
Removes the incorrect and unnecessary txid calculation in `ElectrumX.package_broadcast()`, the daemon will complain anyways if the transactions are invalid.
session: broadcast_package: followup #288
This implements the remainder of the protocol 1.6 changes, and it bumps PROTOCOL_MAX to "1.6". Some of protocol 1.6 was merged previously, see e.g.: - #288 - #302 - kyuupichan/electrumx#1001 - #325 ref spesmilo/electrum-protocol#6 ref #317
Defines a protocol method
blockchain.transaction.broadcast_packagefor protocol version 1.4.4 and implements it so clients can submit a package of transactions to the mempool using the bitcoin coresubmitpackagemethod. This allows clients to broadcast a package of transactions that would on their own not be accepted to the mempool (e.g. below purge fee parent and high fee child to bump)