Adjust tests and readme for signing FirstValid seed

heartbeat/README.md

@@ -126,19 +126,23 @@ The absenteeism mechanism is subject to rare false positives.  The challenge mec
 requires an affirmative response from nodes to indicate they are operating properly on behalf of a
 challenged account.  Both of these needs are addressed by a new transaction type --- _Heartbeat_. A
 Heartbeat transaction contains a signature (`HbProof`) of the blockseed (`HbSeed`) of the
-transaction's firstValid block under the participation key of the account (`HbAddress`) in
+transaction's FirstValid block under the participation key of the account (`HbAddress`) in
 question. Note that the account being heartbeat for is _not_ the `Sender` of the transaction, which
-can be any address.
+can be any address. Signing a recent block seed makes it more difficult to pre-sign heartbeats that
+another machine might send on your behalf. Signing the FirstValid's blockseed (rather than
+FirstValid-1) simply enforces a best practice: emit a transaction with FirstValid set to a committed
+round, not a future round, avoiding a race. The node you send transactions to might not have
+committed your latest round yet.
 
 It is relatively easy for a bad actor to emit Heartbeats for its accounts without actually
 participating. However, there is no financial incentive to do so.  Pretending to be operational when
 offline does not earn block payouts.  Furthermore, running a server to monitor the block chain to
 notice challenges and gather the recent blockseed is not significantly cheaper that simply running a
 functional node. It is _already_ possible for malicious, well-resourced accounts to cause consensus
-difficulties by going online without actually participating.  Heartbeats do not mitigate that
-risk. But these mechanisms have been designed to avoid _motivating_ such behavior, so that they can
-accomplish their actual goal of noticing poor behavior stemming from _inadvertent_ operational
-problems.
+difficulties by putting significant stake online without actually participating.  Heartbeats do not
+mitigate that risk. But these mechanisms have been designed to avoid _motivating_ such behavior, so
+that they can accomplish their actual goal of noticing poor behavior stemming from _inadvertent_
+operational problems.
 
 ## Free Heartbeats
 

ledger/heartbeat_test.go

@@ -34,7 +34,7 @@ import (
    a more realistic ledger. */
 
 // TestHearbeat exercises heartbeat transactions
-func TestHeartBeat(t *testing.T) {
+func TestHeartbeat(t *testing.T) {
 	partitiontest.PartitionTest(t)
 	t.Parallel()
 
@@ -48,6 +48,8 @@ func TestHeartBeat(t *testing.T) {
 			dl := NewDoubleLedger(t, genBalances, cv, cfg)
 			defer dl.Close()
 
+			dl.txns() // tests involving seed are easier if we have the first block in ledger
+
 			// empty HbAddress means ZeroAddress, and it's not online
 			dl.txn(&txntest.Txn{Type: "hb", Sender: addrs[1]},
 				"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAY5HFKQ has no voting keys")
@@ -57,17 +59,18 @@ func TestHeartBeat(t *testing.T) {
 				addrs[2].String()+" has no voting keys")
 
 			// addrs[1] is online, it has voting keys, but seed is missing
-			dl.txn(&txntest.Txn{Type: "hb", Sender: addrs[1], HbAddress: addrs[1]},
-				"does not match round 0's seed")
+			dl.txn(&txntest.Txn{Type: "hb", Sender: addrs[1], HbAddress: addrs[1], FirstValid: 1},
+				"does not match round 1's seed")
 
 			// NewTestGenesis creates random VoterID. Verification will fail.
-			b0, err := dl.generator.BlockHdr(0)
+			b1, err := dl.generator.BlockHdr(1)
 			require.NoError(t, err)
 			dl.txn(&txntest.Txn{
-				Type:      "hb",
-				Sender:    addrs[1],
-				HbAddress: addrs[1],
-				HbSeed:    b0.Seed,
+				Type:       "hb",
+				Sender:     addrs[1],
+				HbAddress:  addrs[1],
+				HbSeed:     b1.Seed,
+				FirstValid: 1,
 			},
 				"heartbeat failed verification with")
 
@@ -84,61 +87,68 @@ func TestHeartBeat(t *testing.T) {
 			})
 
 			// Supply and sign the wrong HbSeed
+			wrong := b1.Seed
+			wrong[0]++
 			dl.txn(&txntest.Txn{
-				Type:      "hb",
-				Sender:    addrs[1],
-				HbAddress: addrs[1],
-				HbSeed:    b0.Seed,
-				HbProof:   otss.Sign(firstID, b0.Seed).ToHeartbeatProof(),
+				Type:       "hb",
+				Sender:     addrs[1],
+				HbAddress:  addrs[1],
+				HbSeed:     wrong,
+				HbProof:    otss.Sign(firstID, wrong).ToHeartbeatProof(),
+				FirstValid: 1,
 			},
 				"does not match round 1's seed")
 
-			b1, err := dl.generator.BlockHdr(1)
+			b2, err := dl.generator.BlockHdr(2)
 			require.NoError(t, err)
 
 			// Supply the right seed, but sign something else. We're also now
 			// setting LastValid and the proper OneTimeIDForRound, so that these
 			// tests are failing for the reasons described, not that.
 			dl.txn(&txntest.Txn{
-				Type:      "hb",
-				LastValid: 30,
-				Sender:    addrs[1],
-				HbAddress: addrs[1],
-				HbSeed:    b1.Seed,
-				HbProof:   otss.Sign(basics.OneTimeIDForRound(30, kd), b0.Seed).ToHeartbeatProof(),
+				Type:       "hb",
+				LastValid:  30,
+				Sender:     addrs[1],
+				HbAddress:  addrs[1],
+				HbSeed:     b2.Seed,
+				HbProof:    otss.Sign(basics.OneTimeIDForRound(30, kd), wrong).ToHeartbeatProof(),
+				FirstValid: 2,
 			},
 				"failed verification")
 
 			// Sign the right seed, but supply something else
 			dl.txn(&txntest.Txn{
-				Type:      "hb",
-				LastValid: 30,
-				Sender:    addrs[1],
-				HbAddress: addrs[1],
-				HbSeed:    b0.Seed,
-				HbProof:   otss.Sign(basics.OneTimeIDForRound(30, kd), b1.Seed).ToHeartbeatProof(),
+				Type:       "hb",
+				LastValid:  30,
+				Sender:     addrs[1],
+				HbAddress:  addrs[1],
+				HbSeed:     wrong,
+				HbProof:    otss.Sign(basics.OneTimeIDForRound(30, kd), b2.Seed).ToHeartbeatProof(),
+				FirstValid: 2,
 			},
-				"does not match round 1's")
+				"does not match round 2's")
 
 			// Mismatch the last valid and OneTimeIDForRound
 			dl.txn(&txntest.Txn{
-				Type:      "hb",
-				LastValid: 29,
-				Sender:    addrs[1],
-				HbAddress: addrs[1],
-				HbSeed:    b1.Seed,
-				HbProof:   otss.Sign(basics.OneTimeIDForRound(30, kd), b1.Seed).ToHeartbeatProof(),
+				Type:       "hb",
+				LastValid:  29,
+				Sender:     addrs[1],
+				HbAddress:  addrs[1],
+				HbSeed:     b2.Seed,
+				HbProof:    otss.Sign(basics.OneTimeIDForRound(30, kd), b2.Seed).ToHeartbeatProof(),
+				FirstValid: 2,
 			},
 				"failed verification")
 
 			// now we can make a real heartbeat, with a properly signed blockseed
 			dl.txn(&txntest.Txn{
-				Type:      "hb",
-				LastValid: 30,
-				Sender:    addrs[1],
-				HbAddress: addrs[1],
-				HbSeed:    b1.Seed,
-				HbProof:   otss.Sign(basics.OneTimeIDForRound(30, kd), b1.Seed).ToHeartbeatProof(),
+				Type:       "hb",
+				LastValid:  30,
+				Sender:     addrs[1],
+				HbAddress:  addrs[1],
+				HbSeed:     b2.Seed,
+				HbProof:    otss.Sign(basics.OneTimeIDForRound(30, kd), b2.Seed).ToHeartbeatProof(),
+				FirstValid: 2,
 			})
 
 		})

ledger/simple_test.go

@@ -129,7 +129,7 @@ func txn(t testing.TB, ledger *Ledger, eval *eval.BlockEvaluator, txn *txntest.T
 		}
 		return
 	}
-	require.True(t, len(problem) == 0 || problem[0] == "")
+	require.True(t, len(problem) == 0 || problem[0] == "", "Transaction did not fail. Expected: %v", problem)
 }
 
 func txgroup(t testing.TB, ledger *Ledger, eval *eval.BlockEvaluator, txns ...*txntest.Txn) error {