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

WIP: Demonstrate remote signing w/ lightning-signer #1

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

Conversation

ksedgwic
Copy link

@ksedgwic ksedgwic commented Aug 4, 2020

Pull Request Checklist

  • If this is your first time contributing, we recommend you read the Code
    Contribution Guidelines
  • All changes are Go version 1.12 compliant
  • The code being submitted is commented according to Code Documentation and Commenting
  • For new code: Code is accompanied by tests which exercise both
    the positive and negative (error paths) conditions (if applicable)
  • For bug fixes: Code is accompanied by new tests which trigger
    the bug being fixed to prevent regressions
  • Any new logging statements use an appropriate subsystem and
    logging level
  • Code has been formatted with go fmt
  • Protobuf files (lnrpc/**/*.proto) have been formatted with
    make rpc-format and compiled with make rpc
  • For code and documentation: lines are wrapped at 80 characters
    (the tab character should be counted as 8 characters, not 4, as some IDEs do
    per default)
  • Running make check does not fail any tests
  • Running go vet does not report any issues
  • Running make lint does not report any new issues that did not
    already exist
  • All commits build properly and pass tests. Only in exceptional
    cases it can be justifiable to violate this condition. In that case, the
    reason should be stated in the commit message.
  • Commits have a logical structure according to Ideal Git Commit Structure

bhandras and others added 20 commits July 28, 2020 17:57
This commit changes the key derivation algo we use to emulate buckets
similar to bbolt. The issue with prefixing keys with either a bucket or
a value prefix is that the cursor couldn't effectively iterate trough
all keys in a bucket, as it skipped the bucket keys.
While there are multiple ways to fix that issue (eg. two pointers,
iterating value keys then bucket keys, etc), the cleanest is to instead
of prefixes in keys we use a postfix indicating whether a key is a
bucket or a value. This also simplifies all operations where we
(recursively) iterate a bucket and is equivalent with the prefixing key
derivation with the addition that bucket and value keys are now
continous.
… tests

This commit moves makeTestDB to db.go and exports it so that we'll be
able to use this function in other unit tests to make them testable with
etcd if needed.
This commit removes the lock set which was used to only add bucket keys
to the tx predicate while also bumping their mod version.
This was useful to reduce the size of the compare set but wasn't useful
to reduce contention as top level buckets were always in the lock set.
This commit extends compatibility with the bbolt kvdb implementation,
which returns ErrIncompatibleValue in case of a bucket/value key
collision. Furthermore the commit also adds an extra precondition to the
transaction when a key doesn't exist. This is needed as we fix reads to
a snapshot revision and other writers may commit the key otherwise.
kvdb+etcd+tests: change etcd flattened bucket key derivation to make it compatible with bbolt
…nlocker-split

mobile: remember walletunlocker.proto
This value actually isn't read anywhere, since it's no longer used.
Instead, `cfg.Db.Bolt.NoSyncFreeList` is what's evaluated when we go to
open the DB.
In this commit, unify the old and new values for `sync-freelist`, and
also ensure that we don't break behavior for any users that're using the
_old_ value.

To do this, we first rename what was `--db.bolt.no-sync-freelist`, to
`--db.bolt.sync-freelist`. This gets rid of the negation on the config
level, and lets us override that value if the user is specifying the
legacy config option.

In the future, we'll deprecate the old config option, in favor of the
new DB scoped option.
This commit adds an integration test that runs on a Windows virtual
machine on Travis. The tests run inside of a "Git Bash" environment
which supports the same command line syntax as a proper bash but doesn't
have all the tooling installed. Some tools also behave differently on
Windows. Therefore we also have to simplify the command to upload the
logs to termbin and remove the upload to file.io on Windows because both
the find and tar command don't work as expected.
We use the event timestamp of a forwarding event as its primary storage
key. On systems with a bad clock resolution this can lead to collisions
of the events if some of the timestamps are identical. We fix this
problem by shifting the timestamps on the nanosecond level until only
unique values remain.
remotesigner/interface.go Outdated Show resolved Hide resolved
CandleHater and others added 7 commits August 5, 2020 09:58
This corrects the output of the chain notifier RPC error. It has been displayed as: "chain notifier RPC *isstill* in the process of starting"
…-config-fix

config: make bbolt freelist settings mutually exclusive
rpc: add missing space to error message
According to the recent discussion `noseedbackup` is not deprecated.
This change clarifies the message about deprecation.
Also fixes a typo.

Closes lightningnetwork#4499
For security reasons, browsers are limited in the header fields they can
send when opening a WebSocket connection. Specifically, the macaroon
cannot be sent in the Grpc-Metadata-Macaroon header field as that would
be possible for normal REST requests. Instead we only have the special
field "Sec-Websocket-Protocol" that can be used to transport custom
data. We allow the macaroon to be sent there and transform it into a
proper header field for the target request.
We add a new document that shows two examples of how to use the
WebSocket REST API with JavaScript.
@ksedgwic ksedgwic changed the title Demonstrate remote signing w/ lightning-signer WIP: Demonstrate remote signing w/ lightning-signer Aug 6, 2020
guggero and others added 25 commits August 12, 2020 16:49
In this commit, we fix a mistake in the split for the new `peer`
package/struct when instantiating the config needed. The existing code
had the DB's swapped. In this commit, we fix this to pass the remote DB
for generic channeldb access, and the local DB for channel graph access.
If a concurrent call to cancel is made while the notifier is shutting
down, this would cause a panic (close of closed channel) since the
events are not removed from the notification sets.
lnd: properly pass peer remote db for graph, local db for rest
…double-cancel

chainntnfs/txnotifier: remove events on teardown
rpcserver: no manual close of restored channels
This version fixes a race condition related to some of the new coin
selection calls.

Fixes lightningnetwork#4532.
In this commit, we make a new wrapper method around the internal
`WalletController` method to ensure it holds the coin select mutex while
the balance is being computed.
In this commit, we update the hop hint selection to account for the fact
that with MPP, a single payment may consume multiple channels. As is, if
a user only has two 0.5 BTC channels, and tries to make a 1 BTC channel,
then the current logic won't include any hop hints.

To solve this, we first add all the channels which in isolation can
carry the payment in question. We then do another pass that accumulates
channels until either we reach our hop-hint limit, or the total
bandwidth that we've accumulate is greater than 2x the payment amount.
lnrpc/invoicesrpc: extend hop hint selection to account for MPP
btcwallet: update btcwallet version, ensure ConfirmedBalance holds the coin select mutex
In this commit, we update our btcwallet dep to the latest version. This
version includes a bug fix for dust calculation. Without this bug fix,
users would potentially significantly overpay on fees, as dust was
computed using the desired fee of the transaction rather than the min
relay fee.
…-fix

build: update to btcwallet version with dust bug fix
ChannelAnnouncement not encountered by integration tests.
Implemented remote GetChannelBasepoints
* upgraded GetChannelBasepoints debugging

* Added remotesigner.ReadyChannel, improved debugging.
* Added remote sign-remote-commitment

* Replaced option_static_remotekey w/ CommitmentType.
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.

10 participants