Address remaining review comments

Co-authored-by: Aditya Kulkarni <[email protected]>

Dockerfile

@@ -10,16 +10,15 @@
  #  To run container: make docker_image_run
  #  
  #  This will place you into the container where you can run zcashd, zcash-cli, 
- #  lightwalletd ingester, and lightwalletd server etc..
+ #  lightwalletd server etc..
  #
  #  First you need to get zcashd sync to current height on testnet, from outside container:
  #  make docker_img_run_zcashd
  #
  #  Sometimes you need to manually start zcashd for the first time, from insdie the container:
  #  zcashd -printtoconsole   
  #
- #  Once the block height is atleast 280,000 you can go ahead and start lightwalletd components
- #  make docker_img_run_lightwalletd_ingest
+ #  Once the block height is atleast 280,000 you can go ahead and start lightwalletd
  #  make docker_img_run_lightwalletd_insecure_server
  #  
  #  If you need a random bash session in the container, use:
@@ -57,7 +56,7 @@ RUN git clone ${LIGHTWALLETD_URL}
 #ADD . /home
 
 RUN cd ./lightwalletd && make
-RUN /usr/bin/install -c /home/lightwalletd/ingest /home/lightwalletd/server /usr/bin/
+RUN /usr/bin/install -c /home/lightwalletd/server /usr/bin/
 
 # Setup layer for zcashd and zcash-cli binary
 FROM golang:1.11 AS zcash_builder
@@ -95,8 +94,8 @@ RUN mkdir -p /home/$ZCASHD_USER/.zcash/ && \
 USER $ZCASHD_USER
 WORKDIR /home/$ZCASHD_USER/
 
-# Use lightwallet server and ingest binaries from prior layer
-COPY --from=lightwalletd_base /usr/bin/ingest /usr/bin/server /usr/bin/
+# Use lightwallet server binary from prior layer
+COPY --from=lightwalletd_base /usr/bin/server /usr/bin/
 COPY --from=zcash_builder /usr/bin/zcashd /usr/bin/zcash-cli /usr/bin/
 COPY --from=zcash_builder /root/.zcash-params/ /home/$ZCASHD_USER/.zcash-params/
 

Makefile

@@ -75,7 +75,7 @@ docker_img_stop_zcashd:
 
 # Start the lightwalletd server in the zcashdlwd container
 docker_img_run_lightwalletd_insecure_server:
-	docker exec -i zcashdlwd server --very-insecure=true --conf-file /home/zcash/.zcash/zcash.conf --db-path /db/sql.db --log-file /logs/server.log --bind-addr 127.0.0.1:18232
+	docker exec -i zcashdlwd server --no-tls-very-insecure=true --conf-file /home/zcash/.zcash/zcash.conf --log-file /logs/server.log --bind-addr 127.0.0.1:18232
 
 # Remove and delete ALL images and containers in Docker; assumes containers are stopped
 docker_remove_all:
@@ -98,4 +98,4 @@ install:
 
 clean:
 	@echo "clean project..."
-	#rm -f $(PROJECT_NAME)
+	#rm -f $(PROJECT_NAME)

README.md

@@ -27,7 +27,7 @@ This will build the server binary, where you can use the below commands to confi
 Assuming you used `make` to build SERVER:
 
 ```
-./server --very-insecure=true --conf-file /home/zcash/.zcash/zcash.conf --log-file /logs/server.log --bind-addr 127.0.0.1:18232
+./server --no-tls-very-insecure=true --conf-file /home/zcash/.zcash/zcash.conf --log-file /logs/server.log --bind-addr 127.0.0.1:18232
 ```
 
 # Production Usage

cmd/server/main.go

@@ -194,27 +194,20 @@ func main() {
 	}
 
 	// Get the sapling activation height from the RPC
-	saplingHeight, blockHeight, chainName, branchID, err := common.GetSaplingInfo(rpcClient)
-	if err != nil {
-		log.WithFields(logrus.Fields{
-			"error": err,
-		}).Warn("Unable to get sapling activation height")
-	}
-
+	// (this first RPC also verifies that we can communicate with zcashd)
+	saplingHeight, blockHeight, chainName, branchID := common.GetSaplingInfo(rpcClient, log)
 	log.Info("Got sapling height ", saplingHeight, " chain ", chainName, " branchID ", branchID)
 
 	// Initialize the cache
 	cache := common.NewBlockCache(opts.cacheSize)
 
-	stopChan := make(chan bool, 1)
-
 	// Start the block cache importer at cacheSize blocks before current height
 	cacheStart := blockHeight - opts.cacheSize
 	if cacheStart < saplingHeight {
 		cacheStart = saplingHeight
 	}
 
-	go common.BlockIngestor(rpcClient, cache, log, stopChan, cacheStart)
+	go common.BlockIngestor(rpcClient, cache, log, cacheStart)
 
 	// Compact transaction service initialization
 	service, err := frontend.NewLwdStreamer(rpcClient, cache, log)
@@ -244,8 +237,7 @@ func main() {
 		log.WithFields(logrus.Fields{
 			"signal": s.String(),
 		}).Info("caught signal, stopping gRPC server")
-		// Stop the block ingestor
-		stopChan <- true
+		os.Exit(1)
 	}()
 
 	log.Infof("Starting gRPC server on %s", opts.bindAddr)

common/cache.go

@@ -37,24 +37,13 @@ func (c *BlockCache) Add(height int, block *walletrpc.CompactBlock) (error, bool
 	c.mutex.Lock()
 	defer c.mutex.Unlock()
 
-	if c.FirstBlock == -1 && c.LastBlock == -1 {
-		// If this is the first block, prep the data structure
-		c.FirstBlock = height
-		c.LastBlock = height - 1
-	}
-
-	// If we're adding a block in the middle of the cache, remove all
-	// blocks after it, since this might be a reorg, and we don't want
-	// Any outdated blocks returned
-	if height >= c.FirstBlock && height <= c.LastBlock {
-		for i := height; i <= c.LastBlock; i++ {
-			delete(c.m, i)
-		}
-		c.LastBlock = height - 1
+	// If we already have this block or any higher blocks, a reorg
+	// must have occurred; these must be re-added
+	for i := height; i <= c.LastBlock; i++ {
+		delete(c.m, i)
 	}
 
-	// Don't allow out-of-order blocks. This is more of a sanity check than anything
-	// If there is a reorg, then the ingestor needs to handle it.
+	// Detect reorg, ingestor needs to handle it
 	if c.m[height-1] != nil && !bytes.Equal(block.PrevHash, c.m[height-1].hash) {
 		return nil, true
 	}
@@ -72,11 +61,14 @@ func (c *BlockCache) Add(height int, block *walletrpc.CompactBlock) (error, bool
 	}
 
 	c.LastBlock = height
+	if c.FirstBlock < 0 || c.FirstBlock > height {
+		c.FirstBlock = height
+	}
 
-	// If the cache is full, remove the oldest block
-	if c.LastBlock-c.FirstBlock+1 > c.MaxEntries {
+	// remove any blocks that are older than the capacity of the cache
+	for c.FirstBlock <= c.LastBlock-c.MaxEntries {
 		delete(c.m, c.FirstBlock)
-		c.FirstBlock = c.FirstBlock + 1
+		c.FirstBlock++
 	}
 
 	return nil, false
@@ -86,11 +78,7 @@ func (c *BlockCache) Get(height int) *walletrpc.CompactBlock {
 	c.mutex.RLock()
 	defer c.mutex.RUnlock()
 
-	if c.LastBlock == -1 || c.FirstBlock == -1 {
-		return nil
-	}
-
-	if height < c.FirstBlock || height > c.LastBlock {
+	if c.m[height] == nil {
 		return nil
 	}