Basic bisecting schedule logic can be simplified

[milosevic]

In the basic bisection schedule implementation, see

tendermint-rs/light-client/src/components/scheduler.rs

Lines 56 to 75 in 184a1f9

	pub fn basic_bisecting_schedule(
	light_store: &dyn LightStore,
	current_height: Height,
	target_height: Height,
	) -> Height {
	let trusted_height = light_store
	.highest(VerifiedStatus::Verified)
	.map(|lb| lb.height())
	.unwrap();
	if trusted_height == current_height && trusted_height < target_height {
	target_height
	} else if trusted_height < current_height && trusted_height < target_height {
	midpoint(trusted_height, current_height)
	} else if trusted_height == target_height {
	target_height
	} else {
	midpoint(current_height, target_height)
	}
	}

lines 70 to 74 shouldn't be needed.

In my understanding, There are only two scenarios in which basic_bisecting_schedule is called (in case verification of header failed), and they are covered by lines:

tendermint-rs/light-client/src/components/scheduler.rs

Lines 66 to 69 in 184a1f9

	if trusted_height == current_height && trusted_height < target_height {
	target_height
	} else if trusted_height < current_height && trusted_height < target_height {
	midpoint(trusted_height, current_height)

Also trusted_height should always be smaller than target_height.

[ebuchman]

Similar to my comment in #294 (comment), some of which still applies

[romac]

Inlining #294 (comment):

Also a note about the scheduler. It's a bit hard to read the bisection logic out of it.

• Can we use single word variable names like trusted, next, and target to make this more readable in cases like this where it's obvious we're dealing with heights and no other types?
• The conditional block is hard to read. Can we clarify the cases somehow by relying on relationships that should always hold when verifying into the future, like (?) :
  • trusted <= next
  • trusted <= target
  • next <= target
• If the above conditions are true, it's hard to understand that last branch of the conditional (it would never trigger?)
• How to reconcile previous point with need to schedule into the past? Split the schedule function in two ?

[ebuchman]

I think this has been completed? Let me know if not.