Separating out a more generic DA interface

[Manav-Aggarwal]

State of DA Interface in Rollkit

Currently, Rollkit contains the interface described in DA Interface ADR. Extracted implementation:

// StatusCode is a type for DA layer return status.
// TODO: define an enum of different non-happy-path cases
// that might need to be handled by Rollkit independent of
// the underlying DA chain.
type StatusCode uint64

// Data Availability return codes.
const (
	StatusUnknown StatusCode = iota
	StatusSuccess
	StatusTimeout
	StatusError
)

// BaseResult contains basic information returned by DA layer.
type BaseResult struct {
	// Code is to determine if the action succeeded.
	Code StatusCode
	// Message may contain DA layer specific information (like DA block height/hash, detailed error message, etc)
	Message string
	// DAHeight informs about a height on Data Availability Layer for given result.
	DAHeight uint64
}

// ResultSubmitBlock contains information returned from DA layer after block submission.
type ResultSubmitBlock struct {
	BaseResult
	// Not sure if this needs to be bubbled up to other
	// parts of Rollkit.
	// Hash hash.Hash
}

// ResultCheckBlock contains information about block availability, returned from DA layer client.
type ResultCheckBlock struct {
	BaseResult
	// DataAvailable is the actual answer whether the block is available or not.
	// It can be true if and only if Code is equal to StatusSuccess.
	DataAvailable bool
}

// ResultRetrieveBlocks contains batch of blocks returned from DA layer client.
type ResultRetrieveBlocks struct {
	BaseResult
	// Block is the full block retrieved from Data Availability Layer.
	// If Code is not equal to StatusSuccess, it has to be nil.
	Blocks []*types.Block
}

// DataAvailabilityLayerClient defines generic interface for DA layer block submission.
// It also contains life-cycle methods.
type DataAvailabilityLayerClient interface {
	// Init is called once to allow DA client to read configuration and initialize resources.
	Init(namespaceID types.NamespaceID, config []byte, kvStore ds.Datastore, logger log.Logger) error

	// Start is called once, after Init. It's implementation should start operation of DataAvailabilityLayerClient.
	Start() error

	// Stop is called once, when DataAvailabilityLayerClient is no longer needed.
	Stop() error

	// SubmitBlock submits the passed in block to the DA layer.
	// This should create a transaction which (potentially)
	// triggers a state transition in the DA layer.
	SubmitBlock(ctx context.Context, block *types.Block) ResultSubmitBlock

	// CheckBlockAvailability queries DA layer to check data availability of block corresponding at given height.
	CheckBlockAvailability(ctx context.Context, dataLayerHeight uint64) ResultCheckBlock
}

// BlockRetriever is additional interface that can be implemented by Data Availability Layer Client that is able to retrieve
// block data from DA layer. This gives the ability to use it for block synchronization.
type BlockRetriever interface {
	// RetrieveBlocks returns blocks at given data layer height from data availability layer.
	RetrieveBlocks(ctx context.Context, dataLayerHeight uint64) ResultRetrieveBlocks
}

This interface is currently implemented by three DA layers: celestia, mock, grpc, and contexts were introduced in #623.

Proposed more generic DA Interface

Recently, @nashqueue and @Wondertan proposed a more generic DA interface in ADR-009, extracted proposal:

type Proof []byte // In case of Celestia this could be the inclusion proof to the blob 
type Data []byte // This could be a block or a transaction for example
type Ref []byte
type Commit []byte
type DataAvailability interface {
		Commit(Data) (Commit, error)
		Put([]Data) ([]Ref, []Proof, error)
		Get(Ref) (Data, Proof, error)
		Validate(Ref, Proof) (bool, error)
}

Note that this proposal was made without referencing ADR-006 and we should discuss what changes would make sense going from the current interface to a more generic interface with eventually separating out the interface in its own repository and importing it as a dependency to Rollkit.

Points of Discussion

Here's a list of preliminary questions that came up when exploring these changes:

• Do we want to keep lifecycle methods, namely Init, Start, and Stop as part of the DA interface? Currently, lifecycle methods are a part of the interface to enable choosing DA in configuration (file or CLI flags) without changing cosmos-sdk which is where a DA object is initialized when a Rollkit node is created. As we move towards supporting more execution environments, we'd like to enable the creation of a Rollkit node through a new CLI. So alternatively we can accept object implementing DA interface, and leave the lifecycle to the developer. Making lifecycle methods part of the Service Interface is another option mentioned by @liamsi, but I'm not sure what that looks like in practice.
• How do the current arguments of Init, namely namespaceID, config, kvStore, and logger fit into the new design?
• What was the rationale behind separating the retriever from the DALC in ADR-006? Is that separation something we should retain or get rid of? Currently, the block manager contains a separate object for retriever and DALC.
• The method, CheckBlockAvailability, which is part of the DA interface, is never used anywhere in the Rollkit implementation. When ADR-006 was designed, where was the intended use case behind this method and do we still plan to use it somewhere?
• Is the argument to Put - Data just a byte array or is it an array of byte arrays? In my opinion, byte array makes more sense since that is what is returned when we marshal a protobuf object. Are there instances where array of byte arrays is needed?
• Currently, DA height is the equivalent of Refsince it is the only argument to RetrieveBlocks, do we expect the developer to provide a Ref struct that can be Marshaled to and from a byte array for each custom DA implementation?
• Where does the Commit method introduced in the new design fit in with the current implementation?
• Do we wish to retain contexts in the new design in order to make requests be easily cancellable or have deadlines?
• How do the structs StatusCode and BaseResult fit into the new design? If the goal of the more generic interface is to have all arguments and return values be byte arrays and all serialization/deserialization be handled by the custom DA implementation, how do we incorporate this change in manager.go? Currently, manager.go is where SubmitBlock and RetrieveBlocks are called.

Putting each question above as its own comment below.

[Manav-Aggarwal]

Do we want to keep lifecycle methods, namely Init, Start, and Stop as part of the DA interface? Currently, lifecycle methods are a part of the interface to enable choosing DA in configuration (file or CLI flags) without changing cosmos-sdk which is where a DA object is initialized when a Rollkit node is created. As we move towards supporting more execution environments, we'd like to enable the creation of a Rollkit node through a new CLI. So alternatively we can accept object implementing DA interface, and leave the lifecycle to the developer. Making lifecycle methods part of the Service Interface is another option mentioned by @liamsi, but I'm not sure what that looks like in practice.

[liamsi]

I was simply suggesting to add another interface for the lifecycle methods. The concrete implementation may then chose to implement that (e.g. via start/ stop etc as before) or have their own/different mechanism instead.

[Manav-Aggarwal]

How do the current arguments of Init, namely namespaceID, config, kvStore, and logger fit into the new design?

[tzdybal]

There are two main approaches:

1. Rollkit configures/initializes the object in a generic way (current implementation).
2. Everything is configured by the developer and ready/configured & running object implementing only DA interface is passed to Rollkit.

Keeping DA interface clean (only DA methods) is probably better way to go. But IMHO it's implementation detail, whether we pass object that implements DA or kind of "DA Service" which is DA + Service (all the "lifecycle" methods).

I can also imagine that sometimes developers would like to have full control over lifecycle of their objects, which makes option 2 more favorable.

[tzdybal]

It should be easy to move lifecycle handling from Rollkit to cosmos-sdk.

[Manav-Aggarwal]

What was the rationale behind separating the retriever from the DALC in ADR-006? Is that separation something we should retain or get rid of? Currently, the block manager contains a separate object for retriever and DALC.

[tzdybal]

We planned P2P block sync as a feature from the very beginning.
In case of P2P block sync, we only need to prove block availability, without actually retrieving it from DA. So theoretically, when P2P block sync is implemented we don't have to use retriever anymore.

Retriever was introduced for initial/current design where we need to download blocks from DA.

[Manav-Aggarwal]

Don't Full Nodes still need to be able to retrieve blocks from DA? What happens in the case when a malicious aggregator doesn't gossip blocks over P2P and just publishes to the DA layer?

[nashqueue]

In the case of a malicious retriever, the full nodes must retrieve it from DA. You are right on that :).

[Manav-Aggarwal]

The method, CheckBlockAvailability, which is part of the DA interface, is never used anywhere in the Rollkit implementation. When ADR-006 was designed, where was the intended use case behind this method and do we still plan to use it somewhere?

[tzdybal]

We don't check block availability because we always retrieve blocks from DA (so they has to be available).

But when P2P block sync is implemented, we need to check block availability to prove that block was actually available in DA layer.

[nashqueue]

I think this could be part of the verification method inside P2P blocksync which will verify the proof through the DA-Interface. The blocks that are sent over the wire could include that proof which then only needs to be verified.

[Manav-Aggarwal]

Is the argument to Put - Data just a byte array or is it an array of byte arrays? In my opinion, byte array makes more sense since that is what is returned when we marshal a protobuf object. Are there instances where array of byte arrays is needed?

[tzdybal]

For me it's like batching multiple blobs in a single request. I guess this should be atomic - you put all your blobs or nothing (if for example there is no space left in the block).

Please clarify @Wondertan

[Wondertan]

Exactly. Rollkit will likely rely on multiple namespaces, i.e. for txs + ISRs and for header. The DA interface should support sending and including them atomically, thus the array of byte arrays

[Manav-Aggarwal]

Currently, DA height is the equivalent of Refsince it is the only argument to RetrieveBlocks, do we expect the developer to provide a Ref struct that can be Marshaled to and from a byte array for each custom DA implementation?

[tzdybal]

We can't map DA height from current implementation to Ref directly. Right now, Retriever returns all blocks from given DA height. And Ref can is generic, this can be a single blob, or range of blobs, few shares from a blob.

[nashqueue]

The underlying DA-Layer defines the reference. Rollkit does not care how the reference looks; it will be passed around and only used during verification. Rollkit will only need to understand the logic when creating fraud proofs. Then the FN will specify the reference so LN can fetch it.

[Manav-Aggarwal]

Where does the Commit method introduced in the new design fit in with the current implementation?

[tzdybal]

It's the // Hash hash.Hash in the ResultSubmitBlock 😂

When this was created, node didn't exist yet, not to mention commitments, etc, so it was commented out. Also it was before ADR-007.

[Manav-Aggarwal]

Do we wish to retain contexts in the new design in order to make requests be easily cancellable or have deadlines?

[tzdybal]

Make sense to me 👍

[tzdybal]

CC: @Wondertan

[Wondertan]

Yes ofc!

[Manav-Aggarwal]

How do the structs StatusCode and BaseResult fit into the new design? If the goal of the more generic interface is to have all arguments and return values be byte arrays and all serialization/deserialization be handled by the custom DA implementation, how do we incorporate this change in manager.go? Currently, manager.go is where SubmitBlock and RetrieveBlocks are called.

[tzdybal]

We should be able to implement methods like SubmitBlock/RetreiveBlocks using only DA interface methods. Then they can live inside block package. This refactoring will change the place where we "abstract" things.

[rootulp]

Questions

1. Why does Message contain DA block height as a string if there is a separate field for DAHeight? I'm referring to:

	// Message may contain DA layer specific information (like DA block height/hash, detailed error message, etc)
	Message string
	// DAHeight informs about a height on Data Availability Layer for given result.
	DAHeight uint64

2. Does this method signature need a parameter for block? Without a parameter for block, idk what it's checking the availability of.

// CheckBlockAvailability queries DA layer to check data availability of block corresponding at given height.
CheckBlockAvailability(ctx context.Context, dataLayerHeight uint64) ResultCheckBlock

[tzdybal]

Good questions!

1. Why does Message contain DA block height as a string if there is a separate field for DAHeight? I'm referring to:

It doesn't. It's optional and arbitrary. It may contain some DA specific info. But yes, the comment is outdated and confusing. IIRC DAHeight was added after the Message.

 2. Does this method signature need a parameter for block? Without a parameter for block, idk what it's checking the availability of.

This method definitely require refactoring. It was drafted long time ago, when celestia-node was non-existent, and refactored before any endpoint was exposed from it. Also, it's not used in Rollkit in the current state.