search

raiden-network / raiden

Raiden Network
https://developer.raiden.network
Other
1.84k stars 377 forks source link

Improve typing of `TransitionResult.new_state` #6004

Open palango opened 4 years ago

palango commented 4 years ago

In #6003 we found some edge case in the typing of TransitionResult.new_state.

As written in https://github.com/raiden-network/raiden/pull/6003/files#r395565989

The problem is:

  • ChainState can never be None
    • A transfer task is set to None once it is finished.

Probably the right fix is to have the transition result defined with a typevar and in the tasks use Optional

hackaugusto commented 4 years ago

TransitionResult is already a generic:

https://github.com/raiden-network/raiden/blob/572e857c060fd1b08eadb789799ca40331ce61ef/raiden/transfer/architecture.py#L196-L200

Perhaps I misunderstood your comment?

palango commented 4 years ago

Yeah, the problem here is that new_state can be None in some cases (for the transfer state machine) and is non-optional in the top-level state machine.

This leads to these problems: https://github.com/raiden-network/raiden/pull/6003/files#r395546025

hackaugusto commented 4 years ago

Yeah, the problem here is that new_state can be None in some cases (for the transfer state machine) and is non-optional in the top-level state machine.

TransitionResult is generic over T, it can be Optional[InitiatorState] for the transfer state and ChainState for the top-level.