Skip to content

Scoru: produce inclusion proof dead code

Context

@sribaroud and I noticed that this function is a dead code. It's not used in produce_proof (which is used by the rollup node). It's only used by the tests.

So, there's two solution:

  1. Extract the logic of produce_inclusion_proof hidden in produce_proof to put in in the real produce_inclusion_proof
  2. Move the function in Internal_for_tests to still be able to test the verify_inclusion_proof function.

(1.) is the best solution, yes. However, the design of this proof production will change with the EOL (see #3717 (closed)). I started to work (1.) but I realised that I'll will modify the same exact lines in a very short future. So I propose to do (2.), and to tackle (1.) when closing the EOL issue.

To give you an insight on why: produce_proof can produce 2 inclusions proofs:

  1. An inclusion of the lower_level in the inbox.
  2. If there are no more messages in the lower_lvel, it produces an inclusion proof of the upper_level.

But in the lower_level proof production, we don't know where it is in the history. We produce an inclusion proof based on a compare which looks for the level inside each visited cell. For the upper_level proof, we take the successor of the lower_level, so we know the pointer of this cell. In this situation we almost use the same code as in the dead code produce_inclusion_proof. So, this code should use produce_inclusion_proof instead of redefining it.

However, with EOL, we will know the level of each cell. If we are at level X and there are no more messages to read, we will look for at least one message in level X+1 (EOL if empty). So we will only use the produce_inclusion_proof with compare kind.

To sum up, if I want to use the produce_inclusion_proof, I should (if I try to do my job correctly) also create a produce_inclusion_proof dedicated to the case where we search for the level (with compare). But with EOL, we will only have the latter, and the former produce_inclusion_proof could be removed (modulo that we use it for the tests).

Manually testing the MR

🐫

Checklist

  • n/a Document the interface of any function added or modified (see the coding guidelines)
  • n/a Document any change to the user interface, including configuration parameters (see node configuration)
  • Provide automatic testing (see the testing guide).
  • n/a For new features and bug fixes, add an item in the appropriate changelog (docs/protocols/alpha.rst for the protocol and the environment, CHANGES.rst at the root of the repository for everything else).
  • Select suitable reviewers using the Reviewers field below.
  • Select as Assignee the next person who should take action on that MR
Edited by Valentin Chaboche

Merge request reports