search

filecoin-project / specs-actors

DEPRECATED Specification of builtin actors, in the form of executable code.
Other
86 stars 102 forks source link

ProveReplicaUpdate messages fail when they include sectors from multiple deadlines #1577

Closed Stebalien closed 2 years ago

Stebalien commented 2 years ago

The new actors version introduced in the OhSnap upgrade (network version 14) has a bug that prevents "batched" ProveReplicaUpdate messages from succeeding if these batches span multiple deadlines.

Impact

At the moment, no known Filecoin client proves multiple replica updates in a single message so this bug should not have any impact. Furthermore, "problematic" update batches will fail cleanly so this bug cannot be used to corrupt network state.

Details

  1. Specs-actors iterates over all "snap" declarations in https://github.com/filecoin-project/specs-actors/blob/b263592694a83675344c8b519608f53d0d266e3a/actors/builtin/miner/miner_actor.go#L2253.
  2. It then inserts new sectors into a newSectors array in https://github.com/filecoin-project/specs-actors/blob/b263592694a83675344c8b519608f53d0d266e3a/actors/builtin/miner/miner_actor.go#L2335.

Unfortunately, the index (i) into the newSectors array is from the inner loop. If the message updates a sectors in a single deadline, everything will just "work". However, if a message updates sectors in multiple deadlines, subsequent iterations of the outer loop will overwrite sectors in the newSectors array instead of extending it.

Fortunately, any such message will fail cleanly when it hits https://github.com/filecoin-project/specs-actors/blob/b263592694a83675344c8b519608f53d0d266e3a/actors/builtin/miner/miner_actor.go#L2351 because:

  1. The newSectors array is pre-filled with nil.
  2. If multiple deadlines are updated in a single message, each deadline's sub-batch will be strictly smaller than the newSectors array. This means the final element in this array will always be nil.
  3. sectors.Store will return an error if any "sector info" in the passed array is nil.

Therefore, any message that attempts to prove replica updates update across multiple deadlines will fail cleanly when it attempts to "write back" the updated sectors.