starlight icon indicating copy to clipboard operation
starlight copied to clipboard

sdk/state: change open/confirm/payment validations to check against ChannelState() when possible

Open acharb opened this issue 3 years ago • 5 comments

right now validations for open/confirm/payment are checking against the state of the latest agreements. Now that we have an overall ChannelState() enum, we should instead (or possibly also) check against that value

acharb avatar Aug 27 '21 18:08 acharb

Could you link to some validations that would change and be replaced with the new state function?

leighmcculloch avatar Aug 27 '21 18:08 leighmcculloch

I think these might be it:

open validation: https://github.com/stellar/experimental-payment-channels/blob/6521706e5e22064d7d0e98ba7a4f2c6857e44cea/sdk/state/open.go#L226

propose payment: https://github.com/stellar/experimental-payment-channels/blob/6521706e5e22064d7d0e98ba7a4f2c6857e44cea/sdk/state/payment.go#L98

confirm payment: https://github.com/stellar/experimental-payment-channels/blob/6521706e5e22064d7d0e98ba7a4f2c6857e44cea/sdk/state/payment.go#L160

propose close: https://github.com/stellar/experimental-payment-channels/blob/6521706e5e22064d7d0e98ba7a4f2c6857e44cea/sdk/state/close.go#L95

confirm close: https://github.com/stellar/experimental-payment-channels/blob/6521706e5e22064d7d0e98ba7a4f2c6857e44cea/sdk/state/close.go#L124

acharb avatar Aug 27 '21 21:08 acharb

we might want to add the same validation for checking if already closed / closing too 🤔

acharb avatar Aug 27 '21 21:08 acharb

I think the first four spots you linked to make sense and look like good candidates for that. The last spot less so because the guard is checking an authorized close agreement exists before copying it and modifying it for early closure.

leighmcculloch avatar Aug 27 '21 21:08 leighmcculloch

adding next steps for when I or someone jumps back on this.

We'll need to add more granular states for the open steps. @leighmcculloch 's suggestion: https://github.com/stellar/experimental-payment-channels/pull/270#discussion_r698941132 .

Probably 2 PRs to complete:

  1. add the missing states, and assert them in the tests
  2. swap out the validations to use these new channel states

acharb avatar Sep 01 '21 20:09 acharb