Repository: coinbase/rosetta-cli (go)

Issue:
Add AbsoluteReconciliationCoverage to check:data end conditions
While running on small chains a Decred simnet (local-only network used for development with a one-second target block time), `check:data` will sometimes hang indefinitely due to not consistently reaching an end condition.

Using just `end_conditions.tip = true` will sometimes either fail (due to very fast block times) or exit too early without having checked all accounts yet.

Using `end_conditions.reconciliation_coverage = x` will sometimes also cause the check not to terminate due to very fast processing of `check:data` on simnet (it will reconcile a large % of the total accounts before reaching tip).

I'd like to add a `AbsoluteReconciliationCoverage` end condition such that we can specify an absolute % of accounts that should have been reconciled before returning (as opposed to a relative % after it has reached tip). 

Pull request:
[configuration] Support advanced reconciliation end condition
Closes #147 

This PR provides the ability to use custom conditions for the "reconciliation coverage end condition" (as opposed to always being a relative measurement from tip):
```golang
// ReconciliationCoverage is used to add conditions
// to reconciliation coverage for exiting `check:data`.
// All provided conditions must be satisfied before
// the end condition is considered satisfied.
//
// If FromTip, Tip, Height, and AccountCount are not provided,
// `check:data` will halt as soon as coverage surpasses
// Coverage.
type ReconciliationCoverage struct {
	// Coverage is some value [0.0, 1.0] that represents
	// the % of accounts reconciled.
	Coverage float64 `json:"coverage"`

	// FromTip is a boolean indicating if reconciliation coverage
	// should only be measured from tip (i.e. reconciliations
	// performed at or after tip was reached).
	FromTip bool `json:"from_tip,omitempty"`

	// Tip is a boolean indicating that tip must be reached
	// before reconciliation coverage is considered valid.
	Tip bool `json:"tip,omitempty"`

	// Index is an int64 indicating the height that must be
	// reached before reconciliation coverage is considered valid.
	Index *int64 `json:"height,omitempty"`

	// AccountCount is an int64 indicating the number of accounts
	// that must be observed before reconciliation coverage is considered
	// valid.
	AccountCount *int64 `json:"account_count,omitempty"`
}

```

### Changes
- [x] Add comments to `ethereum.ros`
- [x] increase `max_fee` in `ethereum.ros`
- [x] Add more customization to reconciliation end condition
- [x] add note about using DSL to the README
- [x] update [`rosetta-sdk-go@v0.5.1`](https://github.com/coinbase/rosetta-sdk-go/releases/tag/v0.5.1)
- [x] fix asserter logging: https://github.com/coinbase/rosetta-sdk-go/pull/184

Patch:
diff --git a/README.md b/README.md
index d559e38..08e1329 100644
--- a/README.md
+++ b/README.md
@@ -94,6 +94,11 @@ The new Construction API testing framework (first released in `rosetta-cli@v0.5.
 a new design pattern to allow for complex transaction construction orchestration.
 You can read more about the design goals [here](https://community.rosetta-api.org/t/feedback-request-automated-construction-api-testing-improvements/146).
 
+Most teams write their Construction API tests using the
+[Rosetta Constructor DSL](https://github.com/coinbase/rosetta-sdk-go/tree/master/constructor/dsl).
+You can find an example of a file written in this DSL in the examples
+folder ([here](https://github.com/coinbase/rosetta-cli/blob/master/examples/configuration/ethereum.ros)).
+
 ##### Terminology
 When first learning about a new topic, it is often useful to understand the
 hierarchy of concerns. In the automated Construction API tester, this
diff --git a/configuration/configuration.go b/configuration/configuration.go
index 8fa7c93..0908f73 100644
--- a/configuration/configuration.go
+++ b/configuration/configuration.go
@@ -238,11 +238,21 @@ func assertDataConfiguration(config *DataConfiguration) error {
 	}
 
 	if config.EndConditions.ReconciliationCoverage != nil {
-		coverage := *config.EndConditions.ReconciliationCoverage
+		coverage := config.EndConditions.ReconciliationCoverage.Coverage
 		if coverage < 0 || coverage > 1 {
 			return fmt.Errorf("reconciliation coverage %f must be [0.0,1.0]", coverage)
 		}
 
+		index := config.EndConditions.ReconciliationCoverage.Index
+		if index != nil && *index < 0 {
+			return fmt.Errorf("reconciliation coverage height %d must be >= 0", *index)
+		}
+
+		accountCount := config.EndConditions.ReconciliationCoverage.AccountCount
+		if accountCount != nil && *accountCount < 0 {
+			return fmt.Errorf("reconciliation coverage account count %d must be >= 0", *accountCount)
+		}
+
 		if config.BalanceTrackingDisabled {
 			return errors.New(
 				"balance tracking must be enabled for reconciliation coverage end condition",
diff --git a/configuration/types.go b/configuration/types.go
index f662202..902f52d 100644
--- a/configuration/types.go
+++ b/configuration/types.go
@@ -148,8 +148,41 @@ type ConstructionConfiguration struct {
 	Quiet bool `json:"quiet,omitempty"`
 }
 
+// ReconciliationCoverage is used to add conditions
+// to reconciliation coverage for exiting `check:data`.
+// All provided conditions must be satisfied before
+// the end condition is considered satisfied.
+//
+// If FromTip, Tip, Height, and AccountCount are not provided,
+// `check:data` will halt as soon as coverage surpasses
+// Coverage.
+type ReconciliationCoverage struct {
+	// Coverage is some value [0.0, 1.0] that represents
+	// the % of accounts reconciled.
+	Coverage float64 `json:"coverage"`
+
+	// FromTip is a boolean indicating if reconciliation coverage
+	// should only be measured from tip (i.e. reconciliations
+	// performed at or after tip was reached).
+	FromTip bool `json:"from_tip,omitempty"`
+
+	// Tip is a boolean indicating that tip must be reached
+	// before reconciliation coverage is considered valid.
+	Tip bool `json:"tip,omitempty"`
+
+	// Index is an int64 indicating the height that must be
+	// reached before reconciliation coverage is considered valid.
+	Index *int64 `json:"height,omitempty"`
+
+	// AccountCount is an int64 indicating the number of accounts
+	// that must be observed before reconciliation coverage is considered
+	// valid.
+	AccountCount *int64 `json:"account_count,omitempty"`
+}
+
 // DataEndConditions contains all the conditions for the syncer to stop
-// when running check:data.
+// when running check:data. If any one of these conditions is considered
+// true, `check:data` will stop with success.
 type DataEndConditions struct {
 	// Index configures the syncer to stop once reaching a particular block height.
 	Index *int64 `json:"index,omitempty"`
@@ -163,12 +196,9 @@ type DataEndConditions struct {
 	// for Duration seconds.
 	Duration *uint64 `json:"duration,omitempty"`
 
-	// ReconciliationCoverage configures the syncer to stop
-	// once it has reached tip AND some proportion of
-	// all addresses have been reconciled at an index >=
-	// to when tip was first reached. The range of inputs
-	// for this condition are [0.0, 1.0].
-	ReconciliationCoverage *float64 `json:"reconciliation_coverage,omitempty"`
+	// ReconciliationCoverage configures the syncer to stop once it reaches
+	// some level of reconciliation coverage.
+	ReconciliationCoverage *ReconciliationCoverage `json:"reconciliation_coverage,omitempty"`
 }
 
 // DataConfiguration contains all configurations to run check:data.
diff --git a/examples/configuration/bitcoin.json b/examples/configuration/bitcoin.json
index 9896ec5..9afa31d 100644
--- a/examples/configuration/bitcoin.json
+++ b/examples/configuration/bitcoin.json
@@ -373,7 +373,10 @@
   "balance_tracking_disabled": false,
   "coin_tracking_disabled": false,
   "end_conditions": {
-   "reconciliation_coverage": 0.95
+   "reconciliation_coverage": {
+     "coverage":0.95,
+     "from_tip": true
+   }
   },
   "results_output_file": ""
  }
diff --git a/examples/configuration/ethereum.json b/examples/configuration/ethereum.json
index 295ca75..dcb3c13 100644
--- a/examples/configuration/ethereum.json
+++ b/examples/configuration/ethereum.json
@@ -45,7 +45,10 @@
   "balance_tracking_disabled": false,
   "coin_tracking_disabled": false,
   "end_conditions": {
-   "reconciliation_coverage": 0.95
+   "reconciliation_coverage": {
+     "coverage":0.95,
+     "from_tip": true
+   }
   },
   "results_output_file": ""
  }
diff --git a/examples/configuration/ethereum.ros b/examples/configuration/ethereum.ros
index 35a5d0c..2f076f0 100644
--- a/examples/configuration/ethereum.ros
+++ b/examples/configuration/ethereum.ros
@@ -9,6 +9,10 @@ request_funds(1){
       "create_limit":1
     });
   },
+
+  // Create a separate scenario to request funds so that
+  // the address we are using to request funds does not
+  // get rolled back if funds do not yet exist.
   request{
     loaded_account = find_balance({
       "account_identifier": {{random_account.account_identifier}},
@@ -28,6 +32,8 @@ create_account(1){
       "network_identifier": {{network}},
       "public_key": {{key.public_key}}
     });
+
+    // If the account is not saved, the key will be lost!
     save_account({
       "account_identifier": {{account.account_identifier}},
       "keypair": {{key}}
@@ -45,10 +51,14 @@ transfer(10){
         "currency": {{currency}}
       }
     });
-    max_fee = set_variable("42000000000000");
+
+    // Set the recipient_amount as some value <= sender.balance-max_fee
+    max_fee = "84000000000000";
     available_amount = {{sender.balance.value}} - {{max_fee}};
     recipient_amount = random_number({"minimum": "1", "maximum": {{available_amount}}});
     print_message({"recipient_amount":{{recipient_amount}}});
+
+    // Find recipient and construct operations
     sender_amount = 0 - {{recipient_amount}};
     recipient = find_balance({
       "not_account_identifier":[{{sender.account_identifier}}],
@@ -84,19 +94,24 @@ transfer(10){
 }
 
 return_funds(10){
+  // TODO: add suggested fee dry run to ensure account fully cleared
   transfer{
     transfer.network = {"network":"Ropsten", "blockchain":"Ethereum"};
     currency = {"symbol":"ETH", "decimals":18};
-    max_fee = "42000000000000";
+    max_fee = "84000000000000";
     sender = find_balance({
       "minimum_balance":{
         "value": {{max_fee}},
         "currency": {{currency}}
       }
     });
+    
+    // Set the recipient_amount as some sender.balance-max_fee
     available_amount = {{sender.balance.value}} - {{max_fee}};
     print_message({"available_amount":{{available_amount}}});
     sender_amount = 0 - {{available_amount}};
+
+    // Provide a static address as the recipient and construct operations
     faucet = {"address":"0xb41B39479a525AB69e38c701A713D98E3074252c"};
     transfer.confirmation_depth = "1";
     transfer.operations = [
diff --git a/go.mod b/go.mod
index 30e2ee0..4b8b86d 100644
--- a/go.mod
+++ b/go.mod
@@ -3,7 +3,7 @@ module github.com/coinbase/rosetta-cli
 go 1.13
 
 require (
-	github.com/coinbase/rosetta-sdk-go v0.5.1-0.20201006163035-c3ad17e072a1
+	github.com/coinbase/rosetta-sdk-go v0.5.2-0.20201006190307-bf4606611446
 	github.com/fatih/color v1.9.0
 	github.com/jinzhu/copier v0.0.0-20190924061706-b57f9002281a
 	github.com/olekukonko/tablewriter v0.0.2-0.20190409134802-7e037d187b0c
diff --git a/go.sum b/go.sum
index 68d7e4d..02c8019 100644
--- a/go.sum
+++ b/go.sum
@@ -168,6 +168,10 @@ github.com/coinbase/rosetta-sdk-go v0.5.1-0.20201006040540-21c3125fd72d h1:GOfii
 github.com/coinbase/rosetta-sdk-go v0.5.1-0.20201006040540-21c3125fd72d/go.mod h1:QVVeKHWFNb0NyzEY06LxXMAylJkYa7n+Hk03pORr0ws=
 github.com/coinbase/rosetta-sdk-go v0.5.1-0.20201006163035-c3ad17e072a1 h1:feFtQluduAZKZBELfar7mm9XgkFTipct0q49iVww4Lw=
 github.com/coinbase/rosetta-sdk-go v0.5.1-0.20201006163035-c3ad17e072a1/go.mod h1:QVVeKHWFNb0NyzEY06LxXMAylJkYa7n+Hk03pORr0ws=
+github.com/coinbase/rosetta-sdk-go v0.5.1 h1:UGuEt/nMWHvnLmWZqJdoQP+HhWL5/6K04rjhPHhmS2M=
+github.com/coinbase/rosetta-sdk-go v0.5.1/go.mod h1:QVVeKHWFNb0NyzEY06LxXMAylJkYa7n+Hk03pORr0ws=
+github.com/coinbase/rosetta-sdk-go v0.5.2-0.20201006190307-bf4606611446 h1:KbefWZdPOb0yBl5DGV/FfJ95OJMiHxdxiJGLY+EjlT0=
+github.com/coinbase/rosetta-sdk-go v0.5.2-0.20201006190307-bf4606611446/go.mod h1:QVVeKHWFNb0NyzEY06LxXMAylJkYa7n+Hk03pORr0ws=
 github.com/coreos/bbolt v1.3.2/go.mod h1:iRUV2dpdMOn7Bo10OQBFzIJO9kkE559Wcmn+qkEiiKk=
 github.com/coreos/etcd v3.3.10+incompatible/go.mod h1:uF7uidLiAD3TWHmW31ZFd/JWoc32PjwdhPthX9715RE=
 github.com/coreos/go-etcd v2.0.0+incompatible/go.mod h1:Jez6KQU2B/sWsbdaef3ED8NzMklzPG4d5KIOhIy30Tk=


Test patch:
diff --git a/configuration/configuration_test.go b/configuration/configuration_test.go
index 3236594..84610f5 100644
--- a/configuration/configuration_test.go
+++ b/configuration/configuration_test.go
@@ -91,7 +91,9 @@ var (
 			StartIndex:                        &startIndex,
 			StatusPort:                        123,
 			EndConditions: &DataEndConditions{
-				ReconciliationCoverage: &goodCoverage,
+				ReconciliationCoverage: &ReconciliationCoverage{
+					Coverage: goodCoverage,
+				},
 			},
 		},
 	}
@@ -132,13 +134,19 @@ var (
 	invalidReconciliationCoverage = &Configuration{
 		Data: &DataConfiguration{
 			EndConditions: &DataEndConditions{
-				ReconciliationCoverage: &badCoverage,
+				ReconciliationCoverage: &ReconciliationCoverage{
+					Coverage: badCoverage,
+				},
 			},
 		},
 	}
 )
 
 func TestLoadConfiguration(t *testing.T) {
+	var (
+		goodAccountCount = int64(10)
+		badAccountCount  = int64(-10)
+	)
 	var tests = map[string]struct {
 		provided *Configuration
 		expected *Configuration
@@ -255,7 +263,9 @@ func TestLoadConfiguration(t *testing.T) {
 				Data: &DataConfiguration{
 					ReconciliationDisabled: true,
 					EndConditions: &DataEndConditions{
-						ReconciliationCoverage: &goodCoverage,
+						ReconciliationCoverage: &ReconciliationCoverage{
+							Coverage: goodCoverage,
+						},
 					},
 				},
 			},
@@ -266,7 +276,9 @@ func TestLoadConfiguration(t *testing.T) {
 				Data: &DataConfiguration{
 					BalanceTrackingDisabled: true,
 					EndConditions: &DataEndConditions{
-						ReconciliationCoverage: &goodCoverage,
+						ReconciliationCoverage: &ReconciliationCoverage{
+							Coverage: goodCoverage,
+						},
 					},
 				},
 			},
@@ -277,7 +289,60 @@ func TestLoadConfiguration(t *testing.T) {
 				Data: &DataConfiguration{
 					IgnoreReconciliationError: true,
 					EndConditions: &DataEndConditions{
-						ReconciliationCoverage: &goodCoverage,
+						ReconciliationCoverage: &ReconciliationCoverage{
+							Coverage: goodCoverage,
+						},
+					},
+				},
+			},
+			err: true,
+		},
+		"valid reconciliation coverage (with account count)": {
+			provided: &Configuration{
+				Data: &DataConfiguration{
+					EndConditions: &DataEndConditions{
+						ReconciliationCoverage: &ReconciliationCoverage{
+							Coverage:     goodCoverage,
+							AccountCount: &goodAccountCount,
+							Index:        &goodAccountCount,
+						},
+					},
+				},
+			},
+			expected: func() *Configuration {
+				cfg := DefaultConfiguration()
+				cfg.Data.EndConditions = &DataEndConditions{
+					ReconciliationCoverage: &ReconciliationCoverage{
+						Coverage:     goodCoverage,
+						AccountCount: &goodAccountCount,
+						Index:        &goodAccountCount,
+					},
+				}
+
+				return cfg
+			}(),
+		},
+		"invalid reconciliation coverage (with account count)": {
+			provided: &Configuration{
+				Data: &DataConfiguration{
+					EndConditions: &DataEndConditions{
+						ReconciliationCoverage: &ReconciliationCoverage{
+							Coverage:     goodCoverage,
+							AccountCount: &badAccountCount,
+						},
+					},
+				},
+			},
+			err: true,
+		},
+		"invalid reconciliation coverage (with index)": {
+			provided: &Configuration{
+				Data: &DataConfiguration{
+					EndConditions: &DataEndConditions{
+						ReconciliationCoverage: &ReconciliationCoverage{
+							Coverage: goodCoverage,
+							Index:    &badAccountCount,
+						},
 					},
 				},
 			},
diff --git a/pkg/tester/data.go b/pkg/tester/data.go
index d806591..09a9f0e 100644
--- a/pkg/tester/data.go
+++ b/pkg/tester/data.go
@@ -388,13 +388,10 @@ func (t *DataTester) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 // EndAtTipLoop runs a loop that evaluates end condition EndAtTip
 func (t *DataTester) EndAtTipLoop(
 	ctx context.Context,
-	minReconciliationCoverage float64,
 ) {
 	tc := time.NewTicker(EndAtTipCheckInterval)
 	defer tc.Stop()
 
-	firstTipIndex := int64(-1)
-
 	for {
 		select {
 		case <-ctx.Done():
@@ -410,15 +407,7 @@ func (t *DataTester) EndAtTipLoop(
 				continue
 			}
 
-			// If we fall behind tip, we must reset the firstTipIndex.
-			if !atTip {
-				firstTipIndex = int64(-1)
-				continue
-			}
-
-			// If minReconciliationCoverage is less than 0,
-			// we should just stop at tip.
-			if minReconciliationCoverage < 0 {
+			if atTip {
 				t.endCondition = configuration.TipEndCondition
 				t.endConditionDetail = fmt.Sprintf(
 					"Tip: %d",
@@ -427,26 +416,89 @@ func (t *DataTester) EndAtTipLoop(
 				t.cancel()
 				return
 			}
+		}
+	}
+}
+
+// EndReconciliationCoverage runs a loop that evaluates ReconciliationEndCondition
+func (t *DataTester) EndReconciliationCoverage( // nolint:gocognit
+	ctx context.Context,
+	reconciliationCoverage *configuration.ReconciliationCoverage,
+) {
+	tc := time.NewTicker(EndAtTipCheckInterval)
+	defer tc.Stop()
+
+	firstTipIndex := int64(-1)
+
+	for {
+		select {
+		case <-ctx.Done():
+			return
+
+		case <-tc.C:
+			atTip, blockIdentifier, err := t.blockStorage.AtTip(ctx, t.config.TipDelay)
+			if err != nil {
+				log.Printf(
+					"%s: unable to evaluate syncer height or if at tip",
+					err.Error(),
+				)
+				continue
+			}
+
+			// Only check coverage if we are at tip.
+			if reconciliationCoverage.Tip || reconciliationCoverage.FromTip {
+				// If we fall behind tip, we must reset the firstTipIndex.
+				if !atTip {
+					firstTipIndex = int64(-1)
+					continue
+				}
+
+				// Once at tip, we want to consider
+				// coverage. It is not feasible that we could
+				// get high reconciliation coverage at the tip
+				// block, so we take the range from when first
+				// at tip to the current block.
+				if firstTipIndex < 0 {
+					firstTipIndex = blockIdentifier.Index
+				}
+			}
+
+			// Check if at required minimum index
+			if reconciliationCoverage.Index != nil && *reconciliationCoverage.Index < blockIdentifier.Index {
+				continue
+			}
+
+			// Check if account count is above minimum index
+			if reconciliationCoverage.AccountCount != nil {
+				allAccounts, err := t.balanceStorage.GetAllAccountCurrency(ctx)
+				if err != nil {
+					log.Printf(
+						"%s: unable to get account count",
+						err.Error(),
+					)
+					continue
+				}
+
+				if int64(len(allAccounts)) < *reconciliationCoverage.AccountCount {
+					continue
+				}
+			}
 
-			// Once at tip, we want to consider
-			// coverage. It is not feasible that we could
-			// get high reconciliation coverage at the tip
-			// block, so we take the range from when first
-			// at tip to the current block.
-			if firstTipIndex < 0 {
-				firstTipIndex = blockIdentifier.Index
+			coverageIndex := int64(0)
+			if reconciliationCoverage.FromTip {
+				coverageIndex = firstTipIndex
 			}
 
-			coverage, err := t.balanceStorage.ReconciliationCoverage(ctx, firstTipIndex)
+			coverage, err := t.balanceStorage.ReconciliationCoverage(ctx, coverageIndex)
 			if err != nil {
 				log.Printf(
-					"%s: unable to get reconciliations coverage",
+					"%s: unable to get reconciliation coverage",
 					err.Error(),
 				)
 				continue
 			}
 
-			if coverage >= minReconciliationCoverage {
+			if coverage >= reconciliationCoverage.Coverage {
 				t.endCondition = configuration.ReconciliationCoverageEndCondition
 				t.endConditionDetail = fmt.Sprintf(
 					"Coverage: %f%%",
@@ -495,7 +547,7 @@ func (t *DataTester) WatchEndConditions(
 
 	if endConds.Tip != nil && *endConds.Tip {
 		// runs a go routine that ends when reaching tip
-		go t.EndAtTipLoop(ctx, -1)
+		go t.EndAtTipLoop(ctx)
 	}
 
 	if endConds.Duration != nil && *endConds.Duration != 0 {
@@ -504,7 +556,7 @@ func (t *DataTester) WatchEndConditions(
 	}
 
 	if endConds.ReconciliationCoverage != nil {
-		go t.EndAtTipLoop(ctx, *endConds.ReconciliationCoverage)
+		go t.EndReconciliationCoverage(ctx, endConds.ReconciliationCoverage)
 	}
 
 	return nil