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

Implementation and spec divergence #298

Closed
danwt opened this issue Aug 25, 2022 · 2 comments
Closed

Implementation and spec divergence #298

danwt opened this issue Aug 25, 2022 · 2 comments
Labels
type: bug Issues that need priority attention -- something isn't working

Comments

@danwt
Copy link
Contributor

danwt commented Aug 25, 2022

The spec at cosmos/ibc@f20ba86
allows different behavior than the implementation.

Consider onRecvVSCMaturedPacket

https://github.com/cosmos/ibc/blob/0a43605fe869e1cf004103dd0f6994e9d8b81055/spec/app/ics-028-cross-chain-validation/methods.md?plain=1#L134-L146

Consider the following

Execution 1 (Spec)

  • Provider handles VSCMatured packet
  • The unbonding validator becomes unbonded
  • Provider handles Slash packet
  • The slash is ignored due to
    // make sure the validator is not yet unbonded;
    // stakingKeeper.Slash() panics otherwise
    if !found || validator.IsUnbonded() {
    // TODO add warning log message
    // fmt.Sprintf("consumer chain %s trying to slash unbonded validator %s", chainID, consAddr.String())
    return false, nil
    }
  • The validator receives more delegations
  • Provider EndBlock occurs, and validator becomes bonded

At this point EndBlock commits a state where the validator is bonded.

Execution 2 (SUT)

However, a different result is possible in the current SUT, given the same transactions:

  • Provider handles VSCMatured packet
  • The maturity is saved for processing in EndBlock
  • Provider handles Slash packet
  • Validator is jailed
  • Provider EndBlock occurs, and validator is not bonded

At this point EndBlock commits a state where the validator is not bonded.

This is a different result.

@danwt danwt added type: bug Issues that need priority attention -- something isn't working ccv-core labels Aug 25, 2022
@mpoke
Copy link
Contributor

mpoke commented Aug 25, 2022

Re. Execution 1

  • Provider handles VSCMatured packet
  • The unbonding validator becomes unbonded

This is not true. At this point, the state of the validator changes (see here) so it can potentially become UNBONDED in staking.EndBlock (see here).

In the spec, the stakingKeeper.UnbondingCanComplete(op.id) is called on receiving a VSCMaturedPacket (see https://github.com/cosmos/ibc/blob/0a43605fe869e1cf004103dd0f6994e9d8b81055/spec/app/ics-028-cross-chain-validation/methods.md#ccv-pcf-rcvmat1).

@danwt
Copy link
Contributor Author

danwt commented Aug 25, 2022

AH yeah I made a mistake 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug Issues that need priority attention -- something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants