-
Notifications
You must be signed in to change notification settings - Fork 18
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
Channels: Open, List & Close #82
Conversation
This pull request introduces 4 alerts when merging 2ce10c6 into ce7f82c - view on LGTM.com new alerts:
|
@fusion44 please check this PR for merging so that @cstenglein can use it for the WebUI channel management |
app/models/lightning.py
Outdated
|
||
@classmethod | ||
def from_grpc_pending(cls, c) -> "Channel": | ||
print(str(c)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably left accidentally?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thats needed and in use ... its to parse channel info from grpc opening pending channels objects, that are slightly different from already active channels - so a special parsing method was needed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The print statement?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah Ok - thats gone now
app/repositories/ln_impl/lnd.py
Outdated
try: | ||
await lncfg.lnd_stub.ConnectPeer(r) | ||
#print("CONNECTED TO PEER") | ||
#print(str(pubkey)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These commented out print statements could be removed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should be removed now
app/repositories/ln_impl/lnd.py
Outdated
async for response in lncfg.lnd_stub.CloseChannel(request): | ||
#print(str(response)) | ||
# TODO: this is still some bytestring that needs correct convertion to a string txid (ok OK for now) | ||
return str(response.close_pending.txid) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think response.close_pending.txid.hex()
should do the trick.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
testing with hex() now
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok looks good on first test
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you! I'd like to request a few small changes though :) Please see my comments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
implementation just for LND - core lightning still throws an NOT IMPELEMENTED error