Resolver - Just for review - Don't accept #4
Conversation
In this commit, we avoid logging an error when the links associated with a peer are not found within its termination watcher. We do this to prevent a benign log message as the links have already been removed from the switch.
In this commit, we modify the balanceUpdate autopilot signal to update the balance according to what's returned to the WalletBalance callback rather than explicitly tracking the balance. This gives the agent a better sense of what the wallet's balance actually is.
In this commit, we remove the disconnection logic within the chanController when failing to open a channel with a peer. We do this as it's already done within the autopilot agent, where it should be, and because it's possible that we were already connected to this node and we happened to disconnect them anyway.
In this commit, we modify the Node interface to return a set of raw bytes, rather than the full pubkey struct. We do this as within the package, commonly we only require the pubkey bytes for fingerprinting purposes. Before this commit, we were forced to _always_ decompress the pubkey which can be expensive done thousands of times a second.
htlcswitch/resolve.go
Outdated
| const ( | ||
| defaultConfigFilename = "resolve.conf" | ||
| defaultServerAddress = "127.0.0.1:10000" | ||
| defaultTls = false |
There was a problem hiding this comment.
Why did you use tabs instead of spaces here?
| @@ -0,0 +1,193 @@ | |||
|
|
|||
| package htlcswitch | |||
There was a problem hiding this comment.
What about this blank line at the beginning of the file?
There was a problem hiding this comment.
Yeah there are a handful of formatting changes that came up for me (including this line), just run gofmt -w resolve.go and it'll change them automatically.
|
|
||
| func isResolverActive() bool { | ||
| // first see if we have a configuration file at the working directory. If | ||
| // we miss that, the resolver is not active |
There was a problem hiding this comment.
Assuming the resolver config file merges with the main config file, how are we going to check if it's active? Will it be another config option? resolver.active or something like that?
There was a problem hiding this comment.
Yes. Added a TODO for it.
htlcswitch/resolve.go
Outdated
| conn, err := grpc.Dial(cfg.ServerAddr, opts...) | ||
| if err != nil { | ||
| log.Errorf("fail to dial: %v", err) | ||
| return nil, nil, errors.New("fail to open connection to resolver") |
htlcswitch/resolve.go
Outdated
| ctx, cancel := context.WithTimeout(context.Background(), 20*time.Second) | ||
| defer cancel() | ||
| // TODO: add additional attributes needed by the resolver. Like the amount | ||
| // we got, CTLV, etc |
There was a problem hiding this comment.
removed the TODO. Already done
htlcswitch/resolve.go
Outdated
| Hash: hex.EncodeToString(pd.RHash[:]), | ||
| Amount: int64(pd.Amount), | ||
| Timeout: pd.Timeout, | ||
| Heightnow: heightNow, |
There was a problem hiding this comment.
I think we'd want this to be HeightNow with a capital N
There was a problem hiding this comment.
This is how protoc generated it. Leaving it like that for now
htlcswitch/resolve.go
Outdated
| ) | ||
| // if there is an error, fail the link | ||
| if err != nil { | ||
| //l.fail(LinkFailureError{code: ErrInternalError}, |
There was a problem hiding this comment.
Can we get rid of these commented out lines?
| @@ -0,0 +1,16 @@ | |||
| // +build noresolver | |||
There was a problem hiding this comment.
What is this file for? Building without resolver logic?
|
Thanks, looking good to me so far. |
offerm
force-pushed
the
resolver
branch
2 times, most recently
from
1e7de30 to
3fb56b1
Compare
|
Draft PR description:
|
|
Draft commit message, formatted to 72 character width:
|
offerm
force-pushed
the
resolver
branch
3 times, most recently
from
5ec9754 to
65fc23a
Compare
After joining the two forked chains, it is necessary to ensure they both agree on the same best hash before proceeding to UnsafeStart the notifier. This is because when the BitcoindClient starts, it retrieves its best known block then calls GetBlockHeaderVerbose on the hash of the retrieved block. This block could be a reorged block if JoinNodes has not completed sync. If it is the case that the best block retrieved has been reorged out of the chain, GetBlockHeaderVerbose errors because bitcoind sets the number of confirmations to -1 on reorged blocks, and the btcd rpc client panics when parsing a block whose number of confirmations is negative. This parsing error is expected to be fixed, and as a more permanent solution chain backends should ensure that the `best block` they retrieve during startup has not been reorged out of the chain.
This commit prevents an error that I've seen on travis, wherein the test fails because a call to Fatal happens after the test finishes. The root cause is that we call Fatal in a goroutine that is reading from the subscribe graph rpc call. To fix this, we now pass an err chan back into the main test context, where we can receive any errors and fail the test if one comes through.
In this commit, we add a compatibility mode for older version of clightning to ensure that we're able to properly parse all their channel updates. An older version of c-lightning would send out encapsulated onion error message with an additional type byte. This would throw off our parsing as we didn't expect the type byte, and so we always 2 bytes off. In order to ensure that we're able to parse these messages and make adjustments to our path finding, we'll first check to see if the type byte is there, if so, then we'll snip off two bytes from the front and continue with parsing. if the bytes aren't found, then we can proceed as normal and parse the request.
In this commit, we update to the latest versions of btcwallet+neutrino that fix a number of bugs within lnd itself. Namely, we ensure that we no longer print out garbage bytes, properly reconnect btcd after being disconnected, ensure we don't add duplicate utxos, and finally ensure that we always start the rescan from the wallet's initial birthday. Fixes lightningnetwork#1775. Fixes lightningnetwork#1494. Fixes lightningnetwork#444.
…outine-fail lnd_test: Prevent calling Fatal in goroutine
…nce-update autopilot/agent: use updateBalance rather than tracking balance explicitly
…s-found htlcswitch+server: avoid logging error if no links are found within peerTerminationWatcher
…mpat lnwire: add new compatibility parsing more for onion error chan updates
…cryptwallet Start deprecating noencryptwallet
In this commit, we fix a bug in the latest migration that could cause the migration to end in a panic. Additionally, we modify the migration to exit early if the bucket wasn't found, as in this case, no migration is required. Fixes lightningnetwork#1874.
…nf-txn lnwallet+rpc: allow unconfirmed transactions to be returned over RPC, fix SubscribeTransaction unconf for neutrino
…-default channeldb+cmd/lncli: enable backwards pagination of listinvoices
channeldb: don't use KeyN in latest migration
In this commit, we update the build to point to the latest version of neutrino which includes a bug fix for a regression that would cause the daemon to spin when at chain tip attempting to always fetch the next set of headers though it was already fully up to date.
Added missing spaces to Makefile
cmd/lncli: Fixed conflicting payinvoice amount output
…r-new make: clean lnd-debug, lncli-debug and .vendor-new
In this commit, we fix a bug in the latest database migration when migrating from 0.4 to 0.5. There's an issue in bolt db where if one deletes a bucket that has a key with a nil value, it thinks that's a sub bucket and attempts a bucket deletion. This will fail as it's not actually a sub-bucket. We get around this by using a cursor to manually delete items in the bucket. Fixes lightningnetwork#1907.
This commit introduces an optional feature to resolve a hash via an external service. If we receive a payment for which we are the final destination, and yet LookupInvoice returns an error indicating an unknown hash, we can attempt to retrieve the preimage for this hash from the external hash resolver service before failing the htlc. This allows for payment hashes and preimages to be created, stored, and retrieved independently from lnd.
This PR removes the dependency on ExchangeUnion/swap-resolver by bringing the protobug service definition into rpc.proto.
Modify the logic to allow more checks to be done even in case of using a resolver.
the resolver may now be configured to use TLS when making the hash resolve RPC call. The CA file (tls.cert) can be provided as part of the configuration file.
No description provided.