[go: up one dir, main page]

DAL/Node: fix use of proto parameters for traps

What

There are 3 commits:

  • The first one is a simple change of argument: use only the protocol parameter that the function needs.
  • The second one unifies Daemon.Handler.get_constants with Node_context.get_proto_parameters. The semantics of both of these functions is preserved.
  • The third commit uses the updated signature of Node_context.get_proto_parameters to get the protocol parameters at the "right" level when these protocol parameters may be used by the accuser.

The last commit is tricky!

Note: in get_attestable_slots, ideally we want the protocol parameters for the attested level to check whether the incentives flag is on, but we don't have them (due to #7291 (closed)), we only have them for the last finalized head. Also, for trap checking it is natural to consider the level of the shard. That's why it is more clear to use the parameters for the published level. Given that accusations are not valid for the first 10 blocks after migration, I think that we would not have problems, but it's better to double-check. I've also mentioned this here.

The follow-up MR !16624 (merged) obtains the protocol parameters at the same time as updating the plugin, thus ensuring that

  • a minimal number of RPC calls are made
  • by default we get the latest parameters instead of the oldest ones

Addresses one item from #7741 (closed).

Why

Because we need to get the protocol parameters for the right level, depending on how they are used. A recent incident on Nextnet highlighted this issue.

Testing

One can use !16676 (merged) to test this: cherry pick the last two commits.

Edited by Eugen Zalinescu

Merge request reports

Loading