Skip to content
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

Bolt8 transport for electrum protocol #8049

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from
Draft

Bolt8 transport for electrum protocol #8049

wants to merge 4 commits into from

Conversation

ecdsa
Copy link
Member

@ecdsa ecdsa commented Nov 3, 2022

This is a draft work on bolt8 transport.
There are two branches, one for the server, and this one for the client.

How to run a server:
PUBLIC=1 BOLT8_KEYFILE=~/electrumx.b8 COST_SOFT_LIMIT=0 COST_HARD_LIMIT=0 COIN=BitcoinSegwit SERVICES=bolt8://:51003,rpc:// NET=regtest DAEMON_URL=http://doggman:donkey@127.0.0.1:18554 DB_DIRECTORY=$HOME/electrumx_db ./electrumx_server

How to run a client:

$alice -s localhost:51003:b:02c51c2a0105d64e5d05fc36e638e77e3d213346e5dd960d263e9fc35af3bbb6dd

The public key of the server is visible in the log:
INFO:SessionManager:bolt8 pubkey 02c51c2a0105d64e5d05fc36e638e77e3d213346e5dd960d263e9fc35af3bbb6dd

Two environment variables are added to electrumx: PUBLIC and WATCHTOWER
They cannot be set at the same time.

If you do not set PUBLIC, the client needs to be whitelisted using electrumx_rpc:
./electrumx_rpc add_user 020f1a5ba32fa3ed098cd0538de5753681f4193c0dd8331415e226c8b8a2220d87

 - prepare for use in ElectrumX:
 - use create_server and create_connection
 - subclass aiorpcx.RSTransport
 - use random, non-persisted privkey
@ecdsa ecdsa marked this pull request as draft November 3, 2022 12:39
Copy link
Member

@SomberNight SomberNight left a comment

Choose a reason for hiding this comment

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

(only looked at first three commits so far)

Comment on lines 309 to 311
connector = self.network.proxy or self.network.asyncio_loop
protocol_factory = partial(LNTransport, LNSession, self.node_keypair.privkey, peer_addr=peer_addr)
asyncio_transport, protocol = await connector.create_connection(protocol_factory, peer_addr.host, peer_addr.port)
Copy link
Member

Choose a reason for hiding this comment

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

self.network.proxy is an Optional[dict]. it does not have .create_connection.
Something like this might work:

from .util import MySocksProxy
proxy = MySocksProxy.from_proxy_dict(self.network.proxy) if self.network.proxy else None
connector = proxy or self.network.asyncio_loop

Comment on lines +2517 to +2521
connector = self.proxy or self.loop
protocol_factory = partial(LNTransport, LNSession, privkey, peer_addr=peer_addr)
asyncio_transport, protocol = await connector.create_connection(protocol_factory, peer_addr.host, peer_addr.port)
await protocol.handshake_done.wait()
peer = Peer(self, node_id, transport=client._protocol, is_channel_backup=True)
Copy link
Member

Choose a reason for hiding this comment

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

self.proxy, self.loop, and client are all undefined

await protocol.handshake_done.wait()
return self.session

async def __aexit__(self, exc_type, exc_value, traceback): await self.session.close()
Copy link
Member

Choose a reason for hiding this comment

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

so much whitespace
we should set the linter to be more strict as it did not complain

async def read_data(self, len):
await self._data_received.wait()
chunk = self._data[0:len]
self._data = self._data[len:]
Copy link
Member

Choose a reason for hiding this comment

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

del self._data[:offset]  # much faster than: buffer=buffer[offset:]

(though atm this method is only used during the handshake, so it's not that performance critical, but better to be consistent imo)

@@ -327,6 +327,14 @@ async def setconfig(self, key, value):
self.config.set_key(key, value)
return True

@command('')
async def create_key_for_server(self, server_pubkey:str) -> str:
Copy link
Member

Choose a reason for hiding this comment

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

I find this name confusing. Both the client and the server ultimately has keypair(s). So which one is create_key_for_server creating a keypair for?

How about create_userkey_for_server?

@SomberNight SomberNight changed the title Bolt8 transport Bolt8 transport for electrum protocol Nov 3, 2022
@@ -327,6 +327,14 @@ async def setconfig(self, key, value):
self.config.set_key(key, value)
return True

@command('')
async def create_key_for_server(self, server_pubkey:str) -> str:
" returns a pubkey to add to electrumx, using the 'add_user' RPC"
Copy link
Member

Choose a reason for hiding this comment

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

Should we add import/export functionality? I guess it would be easier and cleaner not to, but tell users instead to generate new keys instead. Maybe this could be mentioned in the docstring.

However, in time I expect users will ask for it. E.g. consider operating a server for your friends: generating a new key requires user-interaction/cooperation. Anyway, I guess it's easier to ignore this for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants