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

Added Avail DA to Rollkit #2

Closed
wants to merge 25 commits into from
Closed

Added Avail DA to Rollkit #2

wants to merge 25 commits into from

Conversation

chandiniv1
Copy link
Owner

No description provided.

@chandiniv1
Copy link
Owner Author

Hi @aterentic-ethernal
Kindly review the code and suggest the changes that have to be made.It would be very helpful
thank you

Copy link
Collaborator

@aterentic-ethernal aterentic-ethernal left a comment

Choose a reason for hiding this comment

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

Some things are much better 👍 I left a few comments, but Ill done another pass once issue with dependencies is resolved, since I cannot buld project atm.

And one more question, why are there so many unrelated commits in PR?

go.mod Outdated Show resolved Hide resolved
da/avail/datasubmit/submitdata.go Outdated Show resolved Hide resolved
da/avail/datasubmit/submitdata.go Outdated Show resolved Hide resolved
da/avail/avail.go Outdated Show resolved Hide resolved
da/avail/avail.go Show resolved Hide resolved
@chandiniv1
Copy link
Owner Author

And one more question, why are there so many unrelated commits in PR?

I pulled the code from upstream main that might be the reason

Copy link
Collaborator

@akhilkumarpilli akhilkumarpilli left a comment

Choose a reason for hiding this comment

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

left some comments, also fix lint issues.

da/avail/avail.go Outdated Show resolved Hide resolved
da/avail/avail.go Outdated Show resolved Hide resolved
// CheckBlockAvailability queries DA layer to check data availability of block.
func (c *DataAvailabilityLayerClient) CheckBlockAvailability(ctx context.Context, dataLayerHeight uint64) da.ResultCheckBlock {

blockNumber := dataLayerHeight
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why new variable?

Copy link
Owner Author

Choose a reason for hiding this comment

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

@akhilkumarpilli ,datalayerheight and blocknumber are two different fields but currently we are considering datalayerheight as blocknumber that's why I have taken a new variable.If there is no necessity of new variable,I'll remove it. Please suggest

Copy link
Collaborator

Choose a reason for hiding this comment

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

For now, you can remove it.

Copy link
Owner Author

Choose a reason for hiding this comment

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

okay akhil

da/avail/avail.go Outdated Show resolved Hide resolved
da/avail/avail.go Outdated Show resolved Hide resolved
da/avail/datasubmit/submitdata.go Outdated Show resolved Hide resolved
da/avail/avail.go Outdated Show resolved Hide resolved
Comment on lines +37 to +40
genesisHash, err := api.RPC.Chain.GetBlockHash(0)
if err != nil {
return err
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

What if genesis height is different? Will it always be zero?

da/avail/datasubmit/submitdata.go Outdated Show resolved Hide resolved
da/avail/datasubmit/submitdata.go Outdated Show resolved Hide resolved
@chandiniv1 chandiniv1 closed this Oct 30, 2023
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.

5 participants