From ce4ca86ca6be1ce6747cb0d563d4595a9ec9a5df Mon Sep 17 00:00:00 2001 From: Andras Banki-Horvath Date: Fri, 8 Jan 2021 15:27:47 +0100 Subject: [PATCH 1/5] kvdb: fix kvdb.Batch to use state reset when using etcd backend --- channeldb/kvdb/etcd/db.go | 12 ------------ channeldb/kvdb/interface.go | 13 +++++++++++-- 2 files changed, 11 insertions(+), 14 deletions(-) diff --git a/channeldb/kvdb/etcd/db.go b/channeldb/kvdb/etcd/db.go index 81835ef0..576591aa 100644 --- a/channeldb/kvdb/etcd/db.go +++ b/channeldb/kvdb/etcd/db.go @@ -304,15 +304,3 @@ func (db *db) Copy(w io.Writer) error { func (db *db) Close() error { return db.cli.Close() } - -// Batch opens a database read/write transaction and executes the function f -// with the transaction passed as a parameter. After f exits, if f did not -// error, the transaction is committed. Otherwise, if f did error, the -// transaction is rolled back. If the rollback fails, the original error -// returned by f is still returned. If the commit fails, the commit error is -// returned. -// -// Batch is only useful when there are multiple goroutines calling it. -func (db *db) Batch(apply func(tx walletdb.ReadWriteTx) error) error { - return db.Update(apply, func() {}) -} diff --git a/channeldb/kvdb/interface.go b/channeldb/kvdb/interface.go index 9ea4ccdf..616e6317 100644 --- a/channeldb/kvdb/interface.go +++ b/channeldb/kvdb/interface.go @@ -44,8 +44,17 @@ func View(db Backend, f func(tx RTx) error, reset func()) error { // Batch is identical to the Update call, but it attempts to combine several // individual Update transactions into a single write database transaction on // an optimistic basis. This only has benefits if multiple goroutines call -// Batch. -var Batch = walletdb.Batch +// Batch. For etcd Batch simply does an Update since combination is more complex +// in that case due to STM retries. +func Batch(db Backend, f func(tx RwTx) error) error { + if extendedDB, ok := db.(ExtendedBackend); ok { + // Since Batch calls handle external state reset, we can safely + // pass in an empty reset closure. + return extendedDB.Update(f, func() {}) + } + + return walletdb.Batch(db, f) +} // Create initializes and opens a database for the specified type. The // arguments are specific to the database type driver. See the documentation From f1831f058136f4407aac9c9beb54338111ad2254 Mon Sep 17 00:00:00 2001 From: Andras Banki-Horvath Date: Wed, 6 Jan 2021 19:40:30 +0100 Subject: [PATCH 2/5] etcd: add support for user specified ports for testing --- channeldb/kvdb/backend.go | 2 +- channeldb/kvdb/config.go | 6 +++++- channeldb/kvdb/etcd/embed.go | 18 ++++++++++++++---- channeldb/kvdb/etcd/fixture_test.go | 2 +- channeldb/kvdb/kvdb_etcd.go | 8 ++++++-- channeldb/kvdb/kvdb_no_etcd.go | 4 +++- lncfg/db.go | 9 +++++++-- sample-lnd.conf | 6 ++++++ 8 files changed, 43 insertions(+), 12 deletions(-) diff --git a/channeldb/kvdb/backend.go b/channeldb/kvdb/backend.go index 851f430e..309f7540 100644 --- a/channeldb/kvdb/backend.go +++ b/channeldb/kvdb/backend.go @@ -253,7 +253,7 @@ func GetTestBackend(path, name string) (Backend, func(), error) { } return db, empty, nil } else if TestBackend == EtcdBackendName { - return GetEtcdTestBackend(path, name) + return GetEtcdTestBackend(path, 0, 0) } return nil, nil, fmt.Errorf("unknown backend") diff --git a/channeldb/kvdb/config.go b/channeldb/kvdb/config.go index 73c1a303..aeeab574 100644 --- a/channeldb/kvdb/config.go +++ b/channeldb/kvdb/config.go @@ -36,7 +36,11 @@ type BoltConfig struct { // EtcdConfig holds etcd configuration. type EtcdConfig struct { - Embedded bool `long:"embedded" description:"Use embedded etcd instance instead of the external one."` + Embedded bool `long:"embedded" description:"Use embedded etcd instance instead of the external one. Note: use for testing only."` + + EmbeddedClientPort uint16 `long:"embedded_client_port" description:"Client port to use for the embedded instance. Note: use for testing only."` + + EmbeddedPeerPort uint16 `long:"embedded_peer_port" description:"Peer port to use for the embedded instance. Note: use for testing only."` Host string `long:"host" description:"Etcd database host."` diff --git a/channeldb/kvdb/etcd/embed.go b/channeldb/kvdb/etcd/embed.go index b4c5d37b..5b744bc0 100644 --- a/channeldb/kvdb/etcd/embed.go +++ b/channeldb/kvdb/etcd/embed.go @@ -61,7 +61,9 @@ func getFreePort() int { // NewEmbeddedEtcdInstance creates an embedded etcd instance for testing, // listening on random open ports. Returns the backend config and a cleanup // func that will stop the etcd instance. -func NewEmbeddedEtcdInstance(path string) (*BackendConfig, func(), error) { +func NewEmbeddedEtcdInstance(path string, clientPort, peerPort uint16) ( + *BackendConfig, func(), error) { + cfg := embed.NewConfig() cfg.Dir = path @@ -69,9 +71,17 @@ func NewEmbeddedEtcdInstance(path string) (*BackendConfig, func(), error) { cfg.MaxTxnOps = 8192 cfg.MaxRequestBytes = 16384 * 1024 - // Listen on random free ports. - clientURL := fmt.Sprintf("127.0.0.1:%d", getFreePort()) - peerURL := fmt.Sprintf("127.0.0.1:%d", getFreePort()) + // Listen on random free ports if no ports were specified. + if clientPort == 0 { + clientPort = uint16(getFreePort()) + } + + if peerPort == 0 { + peerPort = uint16(getFreePort()) + } + + clientURL := fmt.Sprintf("127.0.0.1:%d", clientPort) + peerURL := fmt.Sprintf("127.0.0.1:%d", peerPort) cfg.LCUrls = []url.URL{{Host: clientURL}} cfg.LPUrls = []url.URL{{Host: peerURL}} diff --git a/channeldb/kvdb/etcd/fixture_test.go b/channeldb/kvdb/etcd/fixture_test.go index 0df75d58..2fa0045f 100644 --- a/channeldb/kvdb/etcd/fixture_test.go +++ b/channeldb/kvdb/etcd/fixture_test.go @@ -32,7 +32,7 @@ type EtcdTestFixture struct { func NewTestEtcdInstance(t *testing.T, path string) (*BackendConfig, func()) { t.Helper() - config, cleanup, err := NewEmbeddedEtcdInstance(path) + config, cleanup, err := NewEmbeddedEtcdInstance(path, 0, 0) if err != nil { t.Fatalf("error while staring embedded etcd instance: %v", err) } diff --git a/channeldb/kvdb/kvdb_etcd.go b/channeldb/kvdb/kvdb_etcd.go index 32791bf6..8369a17c 100644 --- a/channeldb/kvdb/kvdb_etcd.go +++ b/channeldb/kvdb/kvdb_etcd.go @@ -37,10 +37,14 @@ func GetEtcdBackend(ctx context.Context, prefix string, // GetEtcdTestBackend creates an embedded etcd backend for testing // storig the database at the passed path. -func GetEtcdTestBackend(path, name string) (Backend, func(), error) { +func GetEtcdTestBackend(path string, clientPort, peerPort uint16) ( + Backend, func(), error) { + empty := func() {} - config, cleanup, err := etcd.NewEmbeddedEtcdInstance(path) + config, cleanup, err := etcd.NewEmbeddedEtcdInstance( + path, clientPort, peerPort, + ) if err != nil { return nil, empty, err } diff --git a/channeldb/kvdb/kvdb_no_etcd.go b/channeldb/kvdb/kvdb_no_etcd.go index 71090f47..373daeb4 100644 --- a/channeldb/kvdb/kvdb_no_etcd.go +++ b/channeldb/kvdb/kvdb_no_etcd.go @@ -22,6 +22,8 @@ func GetEtcdBackend(ctx context.Context, prefix string, // GetTestEtcdBackend is a stub returning nil, an empty closure and an // errEtcdNotAvailable error. -func GetEtcdTestBackend(path, name string) (Backend, func(), error) { +func GetEtcdTestBackend(path string, clientPort, peerPort uint16) ( + Backend, func(), error) { + return nil, func() {}, errEtcdNotAvailable } diff --git a/lncfg/db.go b/lncfg/db.go index e639ece1..ead5b20f 100644 --- a/lncfg/db.go +++ b/lncfg/db.go @@ -83,10 +83,15 @@ func (db *DB) GetBackends(ctx context.Context, dbPath string, if db.Backend == EtcdBackend { if db.Etcd.Embedded { - remoteDB, _, err = kvdb.GetEtcdTestBackend(dbPath, dbName) + remoteDB, _, err = kvdb.GetEtcdTestBackend( + dbPath, db.Etcd.EmbeddedClientPort, + db.Etcd.EmbeddedPeerPort, + ) } else { // Prefix will separate key/values in the db. - remoteDB, err = kvdb.GetEtcdBackend(ctx, networkName, db.Etcd) + remoteDB, err = kvdb.GetEtcdBackend( + ctx, networkName, db.Etcd, + ) } if err != nil { return nil, err diff --git a/sample-lnd.conf b/sample-lnd.conf index db84a01f..419699ce 100644 --- a/sample-lnd.conf +++ b/sample-lnd.conf @@ -984,6 +984,12 @@ litecoin.node=ltcd ; Useful for testing. ; db.etcd.embedded=false +; If non zero, LND will use this as client port for the embedded etcd instance. +; db.etcd.embedded_client_port=1234 + +; If non zero, LND will use this as peer port for the embedded etcd instance. +; db.etcd.embedded_peer_port=1235 + [bolt] ; If true, prevents the database from syncing its freelist to disk. ; db.bolt.nofreelistsync=1 From b2ab5e8af140fb72f9a3979eb6d1d37e52faa014 Mon Sep 17 00:00:00 2001 From: Andras Banki-Horvath Date: Wed, 6 Jan 2021 19:42:08 +0100 Subject: [PATCH 3/5] itests: run etcd itests with generated ports --- Makefile | 2 +- lntest/node.go | 13 +++++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 4b10a2ff..b5f55e29 100644 --- a/Makefile +++ b/Makefile @@ -176,7 +176,7 @@ itest: build-itest itest-only itest-parallel: build-itest @$(call print, "Running tests") rm -rf lntest/itest/*.log lntest/itest/.logs-*; date - EXEC_SUFFIX=$(EXEC_SUFFIX) echo "$$(seq 0 $$(expr $(ITEST_PARALLELISM) - 1))" | xargs -P $(ITEST_PARALLELISM) -n 1 -I {} scripts/itest_part.sh {} $(NUM_ITEST_TRANCHES) $(TEST_FLAGS) + EXEC_SUFFIX=$(EXEC_SUFFIX) echo "$$(seq 0 $$(expr $(ITEST_PARALLELISM) - 1))" | xargs -P $(ITEST_PARALLELISM) -n 1 -I {} scripts/itest_part.sh {} $(NUM_ITEST_TRANCHES) $(TEST_FLAGS) $(ITEST_FLAGS) lntest/itest/log_check_errors.sh unit: btcd diff --git a/lntest/node.go b/lntest/node.go index 73b44acd..1bf009db 100644 --- a/lntest/node.go +++ b/lntest/node.go @@ -298,6 +298,19 @@ func (cfg NodeConfig) genArgs() []string { if cfg.Etcd { args = append(args, "--db.backend=etcd") args = append(args, "--db.etcd.embedded") + args = append( + args, fmt.Sprintf( + "--db.etcd.embedded_client_port=%v", + nextAvailablePort(), + ), + ) + args = append( + args, fmt.Sprintf( + "--db.etcd.embedded_peer_port=%v", + nextAvailablePort(), + ), + ) + args = append(args, "--db.etcd.embedded") } if cfg.FeeURL != "" { From d3cd412137842fb2a0c6ff02ace0c90ae536b93c Mon Sep 17 00:00:00 2001 From: Andras Banki-Horvath Date: Wed, 6 Jan 2021 19:42:29 +0100 Subject: [PATCH 4/5] ci: add etcd itests to our CI (bitcoind only) --- .travis.yml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.travis.yml b/.travis.yml index cd1c1cb7..08040428 100644 --- a/.travis.yml +++ b/.travis.yml @@ -62,6 +62,11 @@ jobs: - bash ./scripts/install_bitcoind.sh - make itest-parallel backend=bitcoind + - name: Bitcoind Integration with etcd (txindex enabled) + script: + - bash ./scripts/install_bitcoind.sh + - make itest-parallel backend=bitcoind etcd=1 + - name: Bitcoind Integration (txindex disabled) script: - bash ./scripts/install_bitcoind.sh From 22711ade3af1b3117cc54b2466df8ec8e4bb3168 Mon Sep 17 00:00:00 2001 From: Andras Banki-Horvath Date: Mon, 11 Jan 2021 22:48:41 +0100 Subject: [PATCH 5/5] cnct: dispatch contract breach after channel close summary is serialized This commit moves the contract breach event dispatch after the channel close summary has been added to the database. This is important otherwise it may occur that we attempt to mark the channel fully closed while the channel close summary is not yet serialized. --- contractcourt/chain_watcher.go | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/contractcourt/chain_watcher.go b/contractcourt/chain_watcher.go index 97aa7852..6cd3c9dc 100644 --- a/contractcourt/chain_watcher.go +++ b/contractcourt/chain_watcher.go @@ -1128,19 +1128,6 @@ func (c *chainWatcher) dispatchContractBreach(spendEvent *chainntnfs.SpendDetail return err } - // With the event processed, we'll now notify all subscribers of the - // event. - c.Lock() - for _, sub := range c.clientSubscriptions { - select { - case sub.ContractBreach <- retribution: - case <-c.quit: - c.Unlock() - return fmt.Errorf("quitting") - } - } - c.Unlock() - // At this point, we've successfully received an ack for the breach // close. We now construct and persist the close summary, marking the // channel as pending force closed. @@ -1182,6 +1169,19 @@ func (c *chainWatcher) dispatchContractBreach(spendEvent *chainntnfs.SpendDetail log.Infof("Breached channel=%v marked pending-closed", c.cfg.chanState.FundingOutpoint) + // With the event processed and channel closed, we'll now notify all + // subscribers of the event. + c.Lock() + for _, sub := range c.clientSubscriptions { + select { + case sub.ContractBreach <- retribution: + case <-c.quit: + c.Unlock() + return fmt.Errorf("quitting") + } + } + c.Unlock() + return nil }