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

[DRAFT] Extend 02-client w/ Conditional Clients #939

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

notbdu
Copy link
Contributor

@notbdu notbdu commented Mar 9, 2023

Modular blockchains break up a logical blockchain into many constituent parts. To accurately represent these chains and also to account for various types of shared security primitives that are coming up, we need to introduce dependencies between clients.

This is a DRAFT spec change. Opening for discussion.

@notbdu notbdu changed the title Extend 02-client w/ Conditional Clients + Bonding [DRAFT] Extend 02-client w/ Conditional Clients + Bonding Mar 9, 2023
For example, we could have a different client for each of the following functional layers of the blockchain stack:
* Data availability
* Sequencing or transaction ordering
* Execution
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For IBC, we only really care about execution. ie the app state of the blockchain.

In the modular world, in order to have a client following execution, do we need a client of the other layers?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The issue from my POV is that to prove execution - you also need to prove sequencing (TO) & data availability (DA) etc.

For example, in the optimistic rollup (ORU) on Celestia case in the simple case you could have:

  • A consensus proof from a Celestia client (TMLC) for proof of DA and TO
  • A fraud proof from an ORU client (optimistic-LC) for proof of execution
    • ORU client depends on Celestia client
    • Accepts headers that and starts a fraud window only when the header block has been persisted to the Celestia network

Also, with new shared security primitives like babylon chain (checkpointing on BTC) - you can have dependencies on a babylon client for proof of security/checkpoint.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in this specific case that you're talking about for ORUs on Celestia, by proving execution are you not also proving DA and TO? since if the data was not available and the txs were not ordered properly (which is verified on the client side), the rollup nodes would reject those txs anyway?

nevertheless, i think proving DA would be necessary since each Celestia rollup can in theory have their own fork choice rule (i.e. 'Sovereign rollups')

Copy link
Contributor Author

@notbdu notbdu Mar 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you don't prove DA before initiating the fraud window you can get the following:

  • Accept header optimistically w/o ensuring data is available
  • Start fraud window
  • Data is withheld so no fraud prover can actually generate a fraud proof
  • Invalid header gets accepted after fraud window period

@@ -536,12 +583,13 @@ Calling `createClient` with the client type and initial consensus state creates
```typescript
function createClient(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function in general is out of date. Since we take in a ClientState now.

I think if we refactored this to follow what ibc-go is doing this can be achieved.

Since the clientDependencies will be fields on the ClientState

@notbdu notbdu changed the title [DRAFT] Extend 02-client w/ Conditional Clients + Bonding [DRAFT] Extend 02-client w/ Conditional Clients Mar 31, 2023
Copy link
Member

@AdityaSripal AdityaSripal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems quite plausible to me. The main thing is that the client handler at the 02-layer will need to authorize access to the clientstore(s) that our client is dependent on. I think we would ideally only give read-only access? And in that case, I don't think any permissioning from the dependencies is required.

It would be helpful if there's a more concrete example/use to work off of here

@AdityaSripal
Copy link
Member

Happy to see the changes are clean and minimal!

Copy link
Contributor

@crodriguezvega crodriguezvega left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, @notbdu. I think it would be good to update your fork with the latest from main, since there have been some updates in ICS 02 since you opened the PR.

And just a naming suggestion: instead of conditional clients, what dependent clients?

Comment on lines +269 to +300
Client types MUST define methods to verify membership and non membership proofs.

```typescript
type verifyMembership = (
height: Height,
delayTimePeriod: uint64,
delayBlockPeriod: uint64,
proof: []byte,
path: []byte,
value: []byte,
) => error

type verifyNonMembership = (
height: Height,
delayTimePeriod: uint64,
delayBlockPeriod: uint64,
proof: []byte,
path: []byte,
) => error
```

Client types MUST define methods to handle client messages for managing state.

```
type verifyClientMessage(clientMsg: ClientMessage) => error

type checkForMisbehaviour(clientMsg: ClientMessage) => bool

type updateStateOnMisbehaviour(clientMsg: ClientMessage)

type updateState(clientMsg: ClientMessage) => []Height
```
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These functions are already defined elsewhere in this spec, should we still repeat them here?

Client types MUST define methods to handle client messages for managing state.

```
type verifyClientMessage(clientMsg: ClientMessage) => error
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function should now also take an extra parameter (dependencies), right? Should we also explain how those dependencies might be used to verify the client message?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants