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

Migrate types from specs-actors #1

Merged
merged 2 commits into from
Sep 3, 2020
Merged

Migrate types from specs-actors #1

merged 2 commits into from
Sep 3, 2020

Conversation

anorth
Copy link
Member

@anorth anorth commented Sep 3, 2020

This is a copy of most of the specs-actors abi package, in preparation for re-use across release branches.

No semantic change intended. Other notes:

  • Lifted the big package out from under abi
  • Split the primitives.go into chain.go and actor.go
  • Does not bring the policy associated with the RegisteredProof types into this repo

@anorth anorth marked this pull request as ready for review September 3, 2020 05:14
type Multiaddrs = []byte

// PeerID is a byte array representing a Libp2p PeerID
type PeerID = []byte
Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not so sure of these last two. I'd slightly prefer we dropped them and used []byte directly. They both appear in miner info state, and hence in constructor params (which are also accessed by power actor).

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm planning to merge as is but we can reevaluate later today.

// This is currently -1 for "disabled".
//
// This is exposed for testing. Do not modify unless you know what you're doing.
CIDInlineLimit = -1
Copy link
Member Author

Choose a reason for hiding this comment

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

@Stebalien I think we should push this down to a member of the cidBuilder, like the codec. I refrained from doing that here.

Copy link
Member

Choose a reason for hiding this comment

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

This is here to make it easy to play with. I'd just leave it. The builder is effectively just a singleton.

Copy link
Member Author

Choose a reason for hiding this comment

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

It's singleton until we want to change this, and then it's not.

func main() {
// Common types
if err := gen.WriteTupleEncodersToFile("./abi/cbor_gen.go", "abi",
abi.PieceInfo{},
Copy link
Contributor

Choose a reason for hiding this comment

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

Comparing with specs actors this is missing SectorID, adding before merge

type Multiaddrs = []byte

// PeerID is a byte array representing a Libp2p PeerID
type PeerID = []byte
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm planning to merge as is but we can reevaluate later today.

abi/sector.go Outdated

// The maximum assignable sector number.
// Raising this would require modifying our AMT implementation.
//goland:noinspection GoUnusedConst
Copy link
Contributor

Choose a reason for hiding this comment

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

Are these here on purpose? I'd rather keep editor specific directives out of the source. Planning to remove but we can revert later today.

)

// Metadata about a seal proof type.
type SealProofInfo struct {
Copy link
Contributor

Choose a reason for hiding this comment

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

Got confused about this at first but as I understand it this is part of not bringing registered proof policy in here. To integrate into specs actors we will have to wrap this inside specs_actors.SealProofInfo along with SectorMaxLifetime and WindowPoStPartitionSectors.

@ZenGround0 ZenGround0 merged commit 247639f into master Sep 3, 2020
@ZenGround0 ZenGround0 deleted the anorth/types branch September 3, 2020 14:54
@anorth anorth mentioned this pull request Sep 4, 2020
ZenGround0 pushed a commit that referenced this pull request Nov 2, 2020
ZenGround0 added a commit that referenced this pull request Nov 2, 2020
Co-authored-by: Alex <445306+anorth@users.noreply.github.com>
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.

3 participants