Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Netting Channel State must be cleared after all unlocks are done #4024

Closed
hackaugusto opened this issue May 8, 2019 · 0 comments · Fixed by #4027
Closed

Netting Channel State must be cleared after all unlocks are done #4024

hackaugusto opened this issue May 8, 2019 · 0 comments · Fixed by #4027
Assignees

Comments

@hackaugusto
Copy link
Contributor

Abstract

Currently the channel is cleared once the partner locksroot is unlocked:

def handle_channel_batch_unlock(
given_channel_state: NettingChannelState, state_change: ContractReceiveChannelBatchUnlock
) -> TransitionResult[NettingChannelState]:
events: List[Event] = list()
new_channel_state: Optional[NettingChannelState] = given_channel_state
# Unlock is allowed by the smart contract only on a settled channel.
# Ignore the unlock if the channel was not closed yet.
if get_status(given_channel_state) == CHANNEL_STATE_SETTLED:
# Once our half of the channel is unlocked we can clean-up the channel
if state_change.participant == given_channel_state.our_state.address:
new_channel_state = None
return TransitionResult(new_channel_state, events)

This logic was correct before #3553 , because the data was not needed, however after the economic batch unlock the channel data has to be preserved until both locksroots are unlocked.

Motivation

Because of that logic, when there are two unlocks to be done one of the nodes will always clear on the first unlock and the second will hit the following assert:

msg = (
f"Can not resolve channel_id for unlock with locksroot {pex(locksroot)} and "
f"partner {pex(partner)}."
)
assert canonical_identifier is not None, msg

Specification

The channel should be cleared only after all unlocks are done.

Backwards Compatibility

This will change the state representation, so a migration is needed to update the channel's state.

@rakanalh rakanalh added this to Backlog in Raiden Client via automation May 10, 2019
@rakanalh rakanalh added this to the Next Release milestone May 10, 2019
@rakanalh rakanalh moved this from Backlog to In progress in Raiden Client May 10, 2019
Raiden Client automation moved this from In progress to Done May 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Raiden Client
  
Done
Development

Successfully merging a pull request may close this issue.

2 participants