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

Implement distributed payload retrieval #1194

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

litt3
Copy link
Contributor

@litt3 litt3 commented Jan 31, 2025

Why are these changes needed?

  • This PR required a change to the signature of the retriever client GetBlob method. Specifically, the previous method signature required access to a BlobHeader, which cannot be constructed by a client, since they do not have the payment data.

Checks

  • I've made sure the tests are passing. Note that there might be a few flaky tests, in that case, please comment that they are not relevant.
  • I've checked the new test coverage and the coverage percentage didn't drop.
  • Testing Strategy
    • Unit tests
    • Integration tests
    • This PR is not tested :(

@litt3 litt3 self-assigned this Jan 31, 2025
@litt3 litt3 marked this pull request as ready for review January 31, 2025 21:58
logger logging.Logger,
distributedPayloadRetrieverConfig DistributedPayloadRetrieverConfig,
ethConfig geth.EthClientConfig,
thegraphConfig thegraph.Config,
Copy link
Contributor

@bxue-l2 bxue-l2 Jan 31, 2025

Choose a reason for hiding this comment

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

I think we no longer require graph node for retrieving the DA node sockets. Instead, we can call a contract, https://github.com/Layr-Labs/eigenlayer-middleware/blob/fe5834371caed60c1d26ab62b5519b0cbdcb42fa/src/SocketRegistry.sol#L17, and use the mapping there to get the IP address, @0x0aa0 is that right?

mainnet: 0x5a3eD432f2De9645940333e4474bBAAB8cf64cf2

preprod: 0x1747ef24dbbb52cB06382d323f455D48dE1AC7fd

testnet: 0x25aFC8944f501545DDB7E7C4C8A0119965AAb166

But that requires changing this part, https://github.com/Layr-Labs/eigenda/blob/master/api/clients/retrieval_client.go#L122

continue
}
if !valid {
pr.logger.Warn("cert is invalid", "blobKey", blobKey.Hex(), "quorumID", quorumID)
Copy link
Contributor

Choose a reason for hiding this comment

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

would commitment is inconsistent be more concrete? invalid cert might mean wrong signature

continue
}
if !valid {
pr.log.Warn("cert is invalid", "blobKey", blobKey.Hex(), "relayKey", relayKey)
Copy link
Contributor

Choose a reason for hiding this comment

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

"commitments are different" is more descriptive

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.

2 participants