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

Unified reader interface for Data Availability providers #2155

Merged
merged 3 commits into from
Feb 23, 2024

Conversation

ganeshvanahalli
Copy link
Contributor

@ganeshvanahalli ganeshvanahalli commented Feb 20, 2024

This PR introduces a generic reader interface for DA providers to implement.
Multiple implementations can be enabled at once but a batch will only be able to invoke one DA provider (the first occurring valid one in the list).

@cla-bot cla-bot bot added the s Automatically added by the CLA bot if the creator of a PR is registered as having signed the CLA. label Feb 20, 2024
@ganeshvanahalli ganeshvanahalli changed the title Unified interface for Data Availability providers Unified reader interface for Data Availability providers Feb 21, 2024
@ganeshvanahalli ganeshvanahalli marked this pull request as ready for review February 21, 2024 20:56
arbnode/inbox_tracker.go Outdated Show resolved Hide resolved
arbstate/inbox.go Outdated Show resolved Hide resolved
arbstate/inbox.go Outdated Show resolved Hide resolved
Copy link
Member

@Tristan-Wilson Tristan-Wilson left a comment

Choose a reason for hiding this comment

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

LGTM

@ganeshvanahalli ganeshvanahalli merged commit 58e4b50 into master Feb 23, 2024
8 checks passed
@ganeshvanahalli ganeshvanahalli deleted the unified-da-provider-interface branch February 23, 2024 15:54
RISHABHAGRAWALZRA added a commit to availproject/avail-nitro-adapter that referenced this pull request Apr 4, 2024
RISHABHAGRAWALZRA added a commit to availproject/avail-nitro-adapter that referenced this pull request Apr 26, 2024
@RISHABHAGRAWALZRA
Copy link

Hi, I realised even with the unified read interface, the inbox tracker is still taking it as separate readers https://github.com/OffchainLabs/nitro/blob/master/arbnode/node.go#L532, can't we take it into a single umbrella?

@ganeshvanahalli
Copy link
Contributor Author

Hi, I realised even with the unified read interface, the inbox tracker is still taking it as separate readers https://github.com/OffchainLabs/nitro/blob/master/arbnode/node.go#L532, can't we take it into a single umbrella?

Hi, we refactored this in 8c256b7 commit of the unified writer interface PR

@RISHABHAGRAWALZRA
Copy link

Hi, why do we have a list of the providers here https://github.com/OffchainLabs/nitro/pull/2155/files#diff-e24cacc680b8e01e6610107edccd3114f681d583376a9ffc9614fa87df314b32R66, if I understand correctly, at a time we will be using a single da provider, or am I missed something?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
s Automatically added by the CLA bot if the creator of a PR is registered as having signed the CLA.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants