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

fix: make things independent of the Multihash code table #524

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

vmx
Copy link
Contributor

@vmx vmx commented May 3, 2022

It should now be possible to specify your own Multihash code
table.

@Stebalien this one is for you. Please let me know if this is something we want to tackle and whether it's the right direction. Ideally I'd like to use the default alloc value as little as possible and make it explicit most of the time.

It currently build, but I haven't even tried to compile the tests yet, as I'd like to get basic feedback, before I start making everything pretty.

It should now be possible to specify your own Multihash code
table.

/// An IPLD blockstore suitable for injection into the FVM.
///
/// The cgo blockstore adapter implements this trait.
pub trait Blockstore {
///
pub trait Blockstore<const S: usize = DEFAULT_MULTIHASH_ALLOC_SIZE> {
Copy link
Member

Choose a reason for hiding this comment

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

Being generic over the code type makes sense, but being generic over the size really isn't worth it:

  1. It will lead to an annoying refactor in the actors (which will have to be backported).
  2. It will conflict with all open PRs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed, it didn't really felt nice anyway. I'll look if I find a better, less intrusive way.

@Stebalien
Copy link
Member

Note: don't spend too much time on this right now. I'd like to see how much of breaking change this would be if we only make these types generic over the code, but we're trying to wrap up M1 ASAP so bubbling breaking changes that can be punted till M2 isn't a priority.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants