1190: better interface for launch-approval #1355
Conversation
| Err(_) => { | ||
| gum::debug!( | ||
| target: LOG_TARGET, | ||
| session = session_index, | ||
| ?relay_parent, | ||
| "Can't obtain SessionInfo" |
There was a problem hiding this comment.
This is really not how the usage was intended, but that was the case before already.
| match get_extended_session_info( | ||
| session_info_provider, | ||
| ctx.sender(), | ||
| block_entry.parent_hash(), |
There was a problem hiding this comment.
the only difference is that we now fetch executor params using parent_hash of relay_block instead of relay_block, but they should be in the same session, right? Otherwise, why would be passing block_entry.session() and not session of parent?
There was a problem hiding this comment.
Actually, why do we use parent_hash here? Shouldn't it be relay_block as in other places
There was a problem hiding this comment.
They don't even have to be in the same session. The more current head is technically better, for reasons Rob pointed out but also because even if those two would at some point (with impl changes) end up being in different sessions, the newer one would still have access to the session of the other, while the opposite might not be true.
There was a problem hiding this comment.
They don't even have to be in the same session. The more current head is technically better, for reasons Rob pointed out but also because even if those two would at some point (with impl changes) end up being in different sessions, the newer one would still have access to the session of the other, while the opposite might not be true.
yes, this sounds right to me.
* Import changes from archieved repo * Revert erroneous changes * Fix more tests * Resolve discussions * Fix MORE tests * approval-voting: launch_approval better interface (#1355) --------- Co-authored-by: Javier Viola <[email protected]> Co-authored-by: ordian <[email protected]> Co-authored-by: ordian <[email protected]>
|
Thanks for this improvement! I've spent quite a bit of time trying to avoid code duplication and preserve its structure, but your approach is much better. |
* Import changes from archieved repo * Revert erroneous changes * Fix more tests * Resolve discussions * Fix MORE tests * approval-voting: launch_approval better interface (#1355) --------- Co-authored-by: Javier Viola <[email protected]> Co-authored-by: ordian <[email protected]> Co-authored-by: ordian <[email protected]>
Instead of passing
Option<ExecutorParams>and failing the task if it'sNone, we can get all the info we need from extended session info.Note: targeting pr #1190.