Merge pull request #1988 from jyellick/issue-1942-master

 Check for state transfer on view change before ordering

bddtests/peer_basic.feature

@@ -1116,3 +1116,67 @@ Feature: lanching 3 peers
     Examples: Composition options
         |          ComposeFile     |
         |   docker-compose-2.yml   |
+
+
+@issue_1942
+#@doNotDecompose
+Scenario: chaincode example02 with 4 peers, stop and start alternates, reverse
+    Given we compose "docker-compose-4-consensus-batch.yml"
+    And I register with CA supplying username "binhn" and secret "7avZQLwcUe9q" on peers:
+                                  | vp0  |
+    And I use the following credentials for querying peers:
+                            | peer |   username  |    secret    |
+                          | vp0  |  test_user0 | MS9qrN8hFjlE |
+                          | vp1  |  test_user1 | jGlNl6ImkuDo |
+                          | vp2  |  test_user2 | zMflqOKezFiA |
+                          | vp3  |  test_user3 | vWdLCE00vJy0 |
+
+    When requesting "/chain" from "vp0"
+    Then I should get a JSON response with "height" = "1"
+
+    When I deploy chaincode "github.com/hyperledger/fabric/examples/chaincode/go/chaincode_example02" with ctor "init" to "vp0"
+                          | arg1 |  arg2 | arg3 | arg4 |
+                          |  a   |  1000 |  b   |   0  |
+    Then I should have received a chaincode name
+    Then I wait up to "60" seconds for transaction to be committed to peers:
+                          | vp0  | vp1 | vp2 | vp3 |
+
+    When I query chaincode "example2" function name "query" with value "a" on peers:
+                          | vp0 | vp1  | vp2 | vp3 |
+    Then I should get a JSON response from peers with "OK" = "1000"
+                          | vp0 |  vp1 | vp2 | vp3 |
+
+    Given I stop peers:
+                          | vp2 |
+    And I register with CA supplying username "test_user3" and secret "vWdLCE00vJy0" on peers:
+                          | vp3  |
+
+    When I invoke chaincode "example2" function name "invoke" on "vp3" "3" times
+                          |arg1|arg2|arg3|
+                          | a  | b  | 1  |
+    Then I should have received a transactionID
+    Then I wait up to "180" seconds for transaction to be committed to peers:
+                          | vp0 | vp1 | vp3 |
+
+    When I query chaincode "example2" function name "query" with value "a" on peers:
+                          | vp0  | vp1 | vp3 |
+    Then I should get a JSON response from peers with "OK" = "997"
+                          | vp0  | vp1 | vp3 |
+
+    Given I start peers:
+                          | vp2  |
+    And I wait "30" seconds
+
+    Given I stop peers:
+                          | vp1  |
+    When I invoke chaincode "example2" function name "invoke" on "vp3" "20" times
+                          |arg1|arg2|arg3|
+                          | a  | b  | 1  |
+    Then I should have received a transactionID
+    Then I wait up to "300" seconds for transaction to be committed to peers:
+                          | vp0  | vp2 | vp3 |
+
+    When I query chaincode "example2" function name "query" with value "a" on peers:
+                          | vp0  | vp2 | vp3 |
+    Then I should get a JSON response from peers with "OK" = "977"
+                          | vp0  | vp2 | vp3 |

consensus/obcpbft/obc-batch.go

@@ -321,11 +321,11 @@ func (op *obcBatch) processMessage(ocMsg *pb.Message, senderHandle *pb.PeerID) e
 			return nil
 		}
 
+		op.logAddTxFromRequest(req)
+		op.reqStore.storeOutstanding(req)
 		if (op.pbft.primary(op.pbft.view) == op.pbft.id) && op.pbft.activeView {
 			return op.leaderProcReq(req)
 		}
-		op.logAddTxFromRequest(req)
-		op.reqStore.storeOutstanding(req)
 		op.startTimerIfOutstandingRequests()
 		return nil
 	} else if pbftMsg := batchMsg.GetPbftMessage(); pbftMsg != nil {
@@ -409,6 +409,11 @@ func (op *obcBatch) ProcessEvent(event events.Event) events.Event {
 		if op.pbft.activeView && (len(op.batchStore) > 0) {
 			return op.sendBatch()
 		}
+	case *Commit:
+		// TODO, this is extremely hacky, but should go away when batch and core are merged
+		res := op.pbft.ProcessEvent(event)
+		op.startTimerIfOutstandingRequests()
+		return res
 	case viewChangedEvent:
 		// Outstanding reqs doesn't make sense for batch, as all the requests in a batch may be processed
 		// in a different batch, but PBFT core can't see through the opaque structure to see this
@@ -420,6 +425,11 @@ func (op *obcBatch) ProcessEvent(event events.Event) events.Event {
 			op.stopBatchTimer()
 		}
 
+		if op.pbft.skipInProgress {
+			// If we're the new primary, but we're in state transfer, we can't trust ourself not to duplicate things
+			op.reqStore.outstandingRequests.empty()
+		}
+
 		op.reqStore.pendingRequests.empty()
 		for i := op.pbft.h + 1; i <= op.pbft.h+op.pbft.L; i++ {
 			if i <= op.pbft.lastExec {
@@ -499,11 +509,13 @@ func (op *obcBatch) getManager() events.Manager {
 func (op *obcBatch) startTimerIfOutstandingRequests() {
 	if op.pbft.skipInProgress || op.pbft.currentExec != nil {
 		// Do not start view change timer if some background event is in progress
+		logger.Debugf("Replica %d not starting timer because skip in progress or current exec", op.pbft.id)
 		return
 	}
 
 	if !op.reqStore.hasNonPending() {
 		// Only start a timer if we are aware of outstanding requests
+		logger.Debugf("Replica %d not starting timer because all outstanding requests are pending", op.pbft.id)
 		return
 	}
 	op.pbft.softStartTimer(op.pbft.requestTimeout, "Batch outstanding requests")

consensus/obcpbft/obc-batch_test.go

@@ -134,14 +134,8 @@ func TestOutstandingReqsIngestion(t *testing.T) {
 	for i, b := range bs {
 		b.manager.Queue() <- nil
 		count := b.reqStore.outstandingRequests.Len()
-		if i == 0 {
-			if count != 0 {
-				t.Errorf("Batch primary should not have the request in its store: %v", b.reqStore.outstandingRequests)
-			}
-		} else {
-			if count != 1 {
-				t.Errorf("Batch backup %d should have the request in its store", i)
-			}
+		if count != 1 {
+			t.Errorf("Batch backup %d should have the request in its store", i)
 		}
 	}
 }
@@ -190,6 +184,8 @@ func TestOutstandingReqsResubmission(t *testing.T) {
 		}
 	}
 
+	tmp := uint64(1)
+	b.pbft.currentExec = &tmp
 	events.SendEvent(b, committedEvent{})
 	execute()