Supernode code quality suggestions

[lucasmt]

interop.go

• commitVerifiedResult() doesn't use its timestamp parameter. Additionally, since all it does is calling i.verifiedDB.Commit(verifiedResult), the function could just be removed in favor of calling Commit() directly.
• decideVerifiedResult() doesn't use its RoundObservation parameter.
• progressInterop() returns a RoundObservation along with its StepOutput, but this return value seems to only be used in the caller to obtain the LastVerifiedTS. Since this can just be obtained directly from the VerifiedDB, the RoundObservation return value could be removed, which would simplify some of the function signatures.
• refreshCurrentL1OnWait() returns (false, nil) on every path, making the return value redundant. Instead, it could just not return anything and have progressAndRecord() return (false, nil) after calling it.
• observeRound() returns an incomplete RoundObservation when it returns with an error. This is not presently a problem since the only caller discards the return value in this case, but it would probably be preferable to return an empty value in this case to prevent partial values from being used accidentally.
• The LastVerified field of RoundObservation is not used anywhere outside of observeRound(), so it could be a local variable instead of part of the struct.
• In verify(), instead of passing view as an argument to verifyFn() and cycleVerifyFn() directly, it is stored in the frontierView field of the Interop struct, which is then read by the functions. It would be preferable to explicitly pass it as a parameter to the functions instead.
• cycleVerifyFn() returns a full Result, but the only field that is used from it is InvalidHeads. Instead of building a full Result, the function could return only the invalid heads.
• applyPendingTransition() sorts invalidations by ChainID before calling invalidateBlock, but it's unclear why this would be necessary. The same applies to applyRewindPlan() when sorting the ChainID list.
• applyRewindPlan() defines a local function recordErr(), but it seems unnecessary compared to simply calling append() directly. Additionally, recordErr() checks if err != nil, but this is already checked previously everywhere it is called.
• checkChainsReady() goes over all the results and records the first error, then returns this error if it's not nil. Instead, it could simply return immediately as soon as it finds an error.

verified_db.go

• In Commit(), the nested if statements for v.initialized and ts != v.lastTimestamp+1 can be combined into a single if v.initialized && ts != v.lastTimestamp+1.
• Commit() and Get() use two different procedures for copying val to a variable outside of the DB transaction (Commit() uses append, while Get() uses copy). It would be better to pick one approach and use it consistently.
• In GetPendingTransition(), val is copied into data, which is then unmarshalled. Instead, val could be unmarshalled directly.
• initLastTimestamp() silently returns no error if the bucket for the verified results doesn't exist. However, it should always be the case that the bucket already exists when initLastTimestamp() is called. If it doesn't, something went wrong and it would be better for the function to return an error.

algo.go

• The blockPerChain type alias slightly obscures the original type map[eth.ChainID]eth.BlockID without adding much information. It might be more clear to just use the original type.
• verifyExecutingMessage() ignores the executingChain and logIdx parameters.
• verifyInteropMessages() first checks the frontierView for the latest block from a given chain, then falls back to the LogsDB if the block is not present in the frontierView. However, by construction, the frontierView should contain a block for every chain, so the fallback is unnecessary. If a block is missing, this should be considered an error.
• The blocksAtTimestamp parameter in verifyInteropMessages() is unnecessary, since the relevant information is already present in frontierView. This also makes the check blockRef.Hash != expectedBlock.Hash unnecessary (the frontierBlock matches the expectedBlock hash by construction, and the case where we fall back to the LogsDB can be removed, as described above).

rewind.go

• In computeRewindTargets(), there is no reason to call earliest() for the finalized head, since we already return an error if the target block is greater than the current finalized.
• RewindToTimestamp() converts the given timestamp to a block number to perform the rewind. However, in InvalidateBlock() this timestamp is computed by converting it from a block number in the first place. If RewindToTimestamp() were replaced by a function that receives the block number instead, this back-and-forth conversion could be avoided.