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

Add offchain package #154

Closed
wants to merge 10 commits into from
Closed

Add offchain package #154

wants to merge 10 commits into from

Conversation

lthibault
Copy link
Contributor

@lthibault lthibault commented Jan 10, 2024

📝 Summary

Begins formal separation of infrastructure and application logic.
Introduces offchain package that defines a top-level Env struct. This struct contains various APIs to off-chain services. Currently this includes only Datastore. Uses IPFS sidecar as test implementation.

Follow-up work:

  1. Define precompiles to interact with Datastore from Solidity (in progress)
  2. Migrate cstore.Engine to Env

📚 References

https://docs.ipfs.tech/concepts/content-addressing
https://github.com/multiformats/cid


  • I have seen and agree to CONTRIBUTING.md

@lthibault lthibault added the enhancement New feature or request label Jan 10, 2024
@lthibault lthibault self-assigned this Jan 10, 2024
@lthibault lthibault marked this pull request as ready for review January 10, 2024 06:29
@lthibault lthibault changed the title WIP. Add content-oriented datastore. Add offchain and datastore packages Jan 10, 2024
@metachris
Copy link
Contributor

Noticed that the tests fail: https://github.com/flashbots/suave-geth/actions/runs/7471328612/job/20331422338?pr=154#step:9:2848

@lthibault
Copy link
Contributor Author

lthibault commented Jan 10, 2024

I wanted to add some additional context on the IPFS sidecar, as I don't think I've done a very good job of explaining myself so far (cc @Ruteri) 🙂

My reasons for proposing this change boil down to the following:

  1. Running an IPFS sidecar is very helpful for me to prototype fast.
  2. I am adding a feature flag to enable this, so it won’t interfere with any dev or prod setups.
  3. We don’t have to keep IPFS. We can discuss when the time comes to enable the feature in prod.

If the above sounds reasonable to you guys (cc @ferranbt ), here is the course I have plotted for myself:

  • Fix the panic in the unit tests. (WIP)
  • Await one final review from you guys (possibly including a quick sync-up call?)
  • Proceed with follow-up PRs

Please LMK if there's any part of this that seems reckless or ill-advised. I am eager to ship, but I don't want to be the proverbial bull in the china shop! 😅

@lthibault
Copy link
Contributor Author

Okay, I think this is ready to go, pending approval.

I'm proceeding with the next PR: implementing precompiles to interact with the datastore.

@lthibault lthibault mentioned this pull request Jan 11, 2024
1 task
@lthibault lthibault changed the title Add offchain and datastore packages Add offchain package Jan 11, 2024
iface "github.com/ipfs/kubo/core/coreiface"
)

type Env struct {
Copy link
Contributor

Choose a reason for hiding this comment

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

Env feels a little non-descriptive, as in it doesn't really convey a meaning (offchain.Env) 🤔

What is this for, eventually?

Copy link
Contributor Author

@lthibault lthibault Jan 11, 2024

Choose a reason for hiding this comment

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

It's meant to contain the "offchain environment", i.e. the collection of APIs and services that we export to SUAPPs for off-chain data/computation. Ordinarily, I'd agree that Env doesn't convey a lot of meaning, but I think it actually is an "offchain environment" in this case.

@ferranbt ferranbt closed this Jun 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants