erigon Prevent panic when changeset is nil

This PR avoids the nil panic in the special case when the changeset is empty during unwinding.

Jul 12 '24 07:07 antonis19

@antonis19 when panic happened? do you have logs/trace?

Jul 12 '24 07:07 AskAlexSharov

@antonis19 when panic happened? do you have logs/trace?

It didn't happen during regular execution, it happened while I was developing because I had forgotten to set u.CurrentBlockNumber in the UnwindState , so it was 0. So this resulted in this loop being empty:

for currentBlock := u.CurrentBlockNumber; currentBlock > u.UnwindPoint; currentBlock-- {

So in normal circumstances this wouldn't happen, but I thought to solid-proof the code to completely prevent the nil panic under any scenario.

Jul 12 '24 08:07 antonis19

In-theory this code must not allow you request unwind to 0:

func (s *Sync) UnwindTo(unwindPoint uint64, reason UnwindReason, tx kv.Tx) error {
	if tx != nil {
		if casted, ok := tx.(state.HasAggTx); ok {
			// protect from too far unwind
			unwindPointWithCommitment, ok, err := casted.AggTx().(*state.AggregatorRoTx).CanUnwindBeforeBlockNum(unwindPoint, tx)
			// Ignore in the case that snapshots are ahead of commitment, it will be resolved later.
			// This can be a problem if snapshots include a wrong chain so it is ok to ignore it.
			if errors.Is(err, state.ErrBehindCommitment) {
				return nil
			}
			if err != nil {
				return err
			}
			if !ok {
				return fmt.Errorf("too far unwind. requested=%d, minAllowed=%d", unwindPoint, unwindPointWithCommitment)
			}
			unwindPoint = unwindPointWithCommitment
		}
	}

help to check - maybe you found some corner-case of this code also.

Jul 12 '24 08:07 AskAlexSharov

The issue wasn't that the unwinding point was 0, the issue was that u.CurrentBlockNumber was 0, due to my not setting it.

unwindState := &UnwindState{ID: stages.HashState, UnwindPoint: blockNr - 1} // Current block number not set
stageState := &StageState{ID: stages.HashState, BlockNumber: blockNr}

Jul 12 '24 08:07 antonis19

@antonis19 it's ok. let's return err instead of warning

Jul 13 '24 18:07 AskAlexSharov