Skip to content

feat: Celestia DA Layer Client implementation - #399

Merged
tzdybal merged 11 commits into
mainfrom
tzdybal/celestia_da
May 25, 2022
Merged

feat: Celestia DA Layer Client implementation#399
tzdybal merged 11 commits into
mainfrom
tzdybal/celestia_da

Conversation

@tzdybal

@tzdybaltzdybal commented May 10, 2022

Copy link
Copy Markdown
Contributor

This PR introduces celestia DA layer client implementation, based on celestia-node RPC.

It was tested manually with ephemeral cluster using latest & greatest celestia-node and celestia-app.

Main components:

  • da/celestia - implementation of DA interface
    • da/celestia/mock - mock celestia-node RPC server (for testing)
  • libs/cnrc - Celestia Node RPC Client. This is a simple abstraction ti hide complexity related to calling HTTP endpoints, serialization, etc. Right now it's tested via da/test. My intention is to extract this a a separate library, because it probably can be reused. Only used methods are implemented.

Resolves: #396

@tzdybaltzdybal added C:data-availability Component: Data Availability layer abstraction C: Astria labels May 10, 2022
@tzdybaltzdybal self-assigned this May 10, 2022
@tzdybal
tzdybalforce-pushed the tzdybal/celestia_da branch 2 times, most recently from 043b5f7 to 6332ab9CompareMay 25, 2022 10:02
@tzdybal
tzdybal marked this pull request as ready for review May 25, 2022 10:05
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

Merging #399 (6332ab9) into main (315e38e) will increase coverage by 0.12%.
The diff coverage is 59.33%.

@@ Coverage Diff @@## main #399 +/- ##
==========================================
+ Coverage 56.62% 56.74% +0.12% 
==========================================
Files 44 49 +5 Lines 7518 7907 +389 ==========================================
+ Hits 4257 4487 +230 - Misses 2655 2781 +126 - Partials 606 639 +33 
Impacted FilesCoverage Δ
da/celestia/mock/server.go51.31% <51.31%> (ø)
da/celestia/celestia.go53.76% <53.76%> (ø)
libs/cnrc/client.go60.97% <60.97%> (ø)
da/celestia/mock/messages.go82.14% <82.14%> (ø)
da/registry/registry.go100.00% <100.00%> (ø)
libs/cnrc/options.go100.00% <100.00%> (ø)
rpc/json/service.go63.80% <0.00%> (-1.85%)⬇️
types/pb/optimint/optimint.pb.go40.27% <0.00%> (+0.18%)⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 315e38e...6332ab9. Read the comment docs.

@tzdybaltzdybal changed the title feat: first draft of Celestia Node RPC Clientfeat: Celestia DA Layer Client implementationMay 25, 2022
@tzdybal
tzdybal requested a review from jbowen93May 25, 2022 10:18

@jbowen93jbowen93 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM. I've tested this against ephemeral cluster myself and tested it with Ethermint. Both work.

Long term we should consider moving libs/cnrc to a separate repo

Edit: Tomasz is of course ahead of me https://github.com/orgs/celestiaorg/discussions/409

@tzdybal
tzdybalforce-pushed the tzdybal/celestia_da branch from cbe358a to 16aa9cfCompareMay 25, 2022 14:35
return da.ResultSubmitBlock{
DAResult: da.DAResult{
Code: da.StatusSuccess,
Message: "tx hash: " + txResponse.TxHash,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It's a bit weird to use this field for everything. but OK for this PR.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Well, TBH it's just a message. We don't need this hash anyways, I just wanted to add something useful, that may be helpful with debugging.

Comment threadlibs/cnrc/client.go
}

// callNamespacedEndpoint fetches result of /namespaced_{type} family of endpoints into result (this should be pointer!)
func (c *Client) callNamespacedEndpoint(ctx context.Context, namespaceID [8]byte, height uint64, endpoint string, result interface{}) error {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ideally, that 8 would be a const or rather using an existing const.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Created follow-up issue (#411) as this is used literally everywhere.

Comment threadlibs/cnrc/types.go Outdated
@tzdybal
tzdybalforce-pushed the tzdybal/celestia_da branch from fab29d3 to ff8fd97CompareMay 25, 2022 15:07
@tzdybal
tzdybal enabled auto-merge (squash) May 25, 2022 15:09
@tzdybal
tzdybal merged commit 0848bfe into mainMay 25, 2022
@tzdybal
tzdybal deleted the tzdybal/celestia_da branch May 25, 2022 15:11
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C:data-availabilityComponent: Data Availability layer abstraction

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Create celestia DA implementation

4 participants

@tzdybal@codecov-commenter@jbowen93@liamsi