Skip to content
This repository has been archived by the owner on Jan 29, 2024. It is now read-only.

Refactor SubnetID and add ChainID to root #109

Merged
merged 8 commits into from
Jun 7, 2023
Merged

Conversation

adlrocha
Copy link
Contributor

@adlrocha adlrocha commented May 31, 2023

Addresses #78

Our previous implementation of SubnetID had two main issues:

  • As we used strings to internal represent the parent of a subnet, it was dependent of the network type prefix t/f which meant that setting a different network type in Lotus and the actors led to really annoying panics/errors.
  • We didn't have a way to discern among different IPC trees. For this, instead of using root to identify the root of an IPC hierarchy, we use the ChainID to "uniquely" identify different IPC trees.

Finally, we've decided that instead of supporting Ethereum addressed for subnet actors, these will be included in the path of a SubnetID through their corresponding f4 address (thus supporting any future alternative addressing that comes to the network).

Next

sdk/src/subnet_id.rs Outdated Show resolved Hide resolved
}
let mut hasher = FnvHasher::default();
Copy link
Contributor

Choose a reason for hiding this comment

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

Ah cool, I didn't know there was a hasher that output u64 directly. I'll switch to using this then.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is a non-cryptographic hash. Really convenient for this cases (and also supported by Go, so even better).

Copy link
Contributor

Choose a reason for hiding this comment

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

It would be good to include some examples in the form of tests. For example https://github.com/tjwebb/fnv-plus has a default bit length of 52, but also has named methods for 32 and 64. It would help users to want to produce the same chain ID on different platforms to include some documentation on what the Rust version uses.

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

Successfully merging this pull request may close these issues.

3 participants