[pfi79]

A NewView record can be followed by a proposal or a commit when the node
restarts before the proposal is decided. The built-in WAL saves the
pre-prepare with truncateTo=true, which drops the NewView record. After a
restart only the proposal pair remains.

LoadNewViewIfApplicable returned nil, the controller started from the older
application metadata, and Restore set View.Number to the proposal's view.
The controller and the view disagreed about the current view and leader.

Walk the WAL backwards to the latest view transition. Return the last
NewView, and stop at a later ViewChange so it keeps precedence. When no
NewView survives, take the view from the trailing ProposedRecord or Commit
and start the controller there.

Add unit tests for the fallback and an integration test that restarts a node
after a NewView and a proposal, then checks that the controller starts in
the restored view.

Fixes #751

[HagarMeir]

Written by Claude (AI) at @HagarMeir's request.

Two points on LoadLastProposalView and setViewAndSeq:

1. The bool isn't needed. The only caller checks found && lastView > newView. When nothing is found the view is 0, and 0 > newView is always false, so found doesn't change behaviour.

2. newDec = 0 is hard-coded, but Restore reads DecisionsInView from the proposal metadata. It's 0 in practice, because the first proposal after a view change has 0 decisions. But nothing reconciles the two values (NewProposer doesn't, since viewNum == view.Number after Restore). If that ever breaks, the controller and the view disagree on the decision count, and with LeaderRotation on they disagree on the leader too. The tests wouldn't catch it: TestSetViewAndSeqUsesTheTrailingProposalView uses seq 0, which already forces dec to 0 before this branch, and the integration test runs without leader rotation.

Suggestion: return the trailing proposal's *protos.ViewMetadata (nil if there isn't one) and take both values from it:

if md != nil && md.GetViewId() > newView {
	newView = md.GetViewId()
	newDec = md.GetDecisionsInView()
}

A commit always follows its ProposedRecord, so the walk can skip commits and read the proposal before it. It would also be good to give the unit test a non-zero seq so it actually checks dec.

[pfi79]

Written by Claude (AI) at @HagarMeir's request.

Two points on LoadLastProposalView and setViewAndSeq:

1. The bool isn't needed. The only caller checks found && lastView > newView. When nothing is found the view is 0, and 0 > newView is always false, so found doesn't change behaviour.
2. newDec = 0 is hard-coded, but Restore reads DecisionsInView from the proposal metadata. It's 0 in practice, because the first proposal after a view change has 0 decisions. But nothing reconciles the two values (NewProposer doesn't, since viewNum == view.Number after Restore). If that ever breaks, the controller and the view disagree on the decision count, and with LeaderRotation on they disagree on the leader too. The tests wouldn't catch it: TestSetViewAndSeqUsesTheTrailingProposalView uses seq 0, which already forces dec to 0 before this branch, and the integration test runs without leader rotation.

Suggestion: return the trailing proposal's *protos.ViewMetadata (nil if there isn't one) and take both values from it:

if md != nil && md.GetViewId() > newView {
	newView = md.GetViewId()
	newDec = md.GetDecisionsInView()
}

A commit always follows its ProposedRecord, so the walk can skip commits and read the proposal before it. It would also be good to give the unit test a non-zero seq so it actually checks dec.

fixed