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

IPC: Implements mir-validator, mir consensus and decouples storage #9492

Closed

Conversation

adlrocha
Copy link
Contributor

@adlrocha adlrocha commented Oct 14, 2022

Related Issues

Proposed Changes

This PR integrates the Mir framework and implements a pBFT consensus algorithm over Mir. Mir is implemented as an alternative consensus algorithm for Lotus. In order to run Lotus with Mir, a new build parameter build.Consensus has been provided.

This PR includes:

  • The implementation in chain/consensus/mir of Mir as a new consensus interface.
  • A new mir-validator process that implements all the mining and block producing logic for Mir. make mir-validator can be used to compile the process. This process is responsible for ordering new transactions and proposing new blocks to the network.
  • New spacenet compile parameters (see build/params_spacenet.go) and a new build.Consensus flag used to determine the type of consensus to use. Using the Mir consensus disables the block production and winningPoSt process from lotus-miner and forces the use of fake randomness for windowPoSt. lotus-miner can still be used (and run) in a Mir Lotus network for storage-specific features (on boarding storage, starting storage and retrieval deals, markets, etc.).

What's next?

We currently don't support the ability for other full nodes in the network to sync with a Mir network. Only validators running the consensus algorithm get new blocks. The next PR will include the syncing protocol (implementing consensus-shipyard/lotus#4)

Copy link
Contributor

@magik6k magik6k left a comment

Choose a reason for hiding this comment

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

Partial first pass

// Note that the private key used to produce the signature cannot be set ("registered") through this interface.
// Storing and using the private key is completely implementation-dependent.
func (c *CryptoManager) Sign(data [][]byte) ([]byte, error) {
signature, err := c.api.WalletSign(context.Background(), c.key, hash(data))
Copy link
Contributor

Choose a reason for hiding this comment

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

Use domain separation tags if at all possible

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You mean for the signature?

Choose a reason for hiding this comment

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

@magik6k Could you provide an example or the rules to understand how to separate domains properly not to create collisions?

import (
ipfslogging "github.com/ipfs/go-log/v2"

mirlogging "github.com/filecoin-project/mir/pkg/logging"
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not move mir to use https://github.com/ipfs/go-log, like every PL Go project?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@matejpavlovic, @dnkolegov to answer this one.

Choose a reason for hiding this comment

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

Mir uses go-log. That's just a wrapper on top of it.

Comment on lines +7 to +9
"github.com/filecoin-project/mir/pkg/pb/checkpointpb"
"github.com/filecoin-project/mir/pkg/pb/commonpb"
"github.com/filecoin-project/mir/pkg/pb/requestpb"
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is mir using protobuf when the entire Filecoin protocol is cbor-based? Protobufs are going to be a pain to deal with in IPLD.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Mir was @matejpavlovic's project before joining PL. The project used protobuf already and this is why we didn't include cbor in the integration. I am pro-cbor, but again, I'll defer this one to @matejpavlovic.

}

// GetValidatorsFromCfg gets the membership config from a file.
func GetValidatorsFromCfg(config string) (*ValidatorSet, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This probably should come from an embedded file in build/

Copy link
Contributor

Choose a reason for hiding this comment

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

(Or I guess in node repo because we write this file? Maybe in metadata datastore)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We didn't include it in build/ because in the future the validator set will be dynamic (and handled on-chain), but I agree that we could use the datastore instead of a config file). We'll look into it. 🙏

chain/store/store.go Outdated Show resolved Hide resolved
return "", nil
}

func (bft *Mir) ValidateBlock(ctx context.Context, b *types.FullBlock) (err error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Where do we verify that a block coming from the network is legit?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This PR adds only support for Mir validators. Validators can only receive blocks through the orderer layer, and they don't broadcast blocks to the network. Support for full-nodes is introduced in the next PR (maybe it would have made sense to merge both, but it was getting huge).

The relevant code for block validation can be found here: https://github.com/consensus-shipyard/lotus/blob/510c6dfc77485aa463cde07a9f9bccee86addc87/chain/consensus/mir/consensus.go#L106

Sorry if we made this a mess, happy to merge PRs and make it a single on, and/or walk you through the code if it helps 🙏

Comment on lines 1 to 2
�e���c�?�*b pi���8[�z� ���Z�SM���cȠ�3��T�
Ǝ9�z�5�4
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't be committed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We use it to run a local mir network fast and do some manual testing. Will remove it before merging. Thanks.

@adlrocha
Copy link
Contributor Author

Closing for now to reduce noise, I don't think there is a plan to merge Mir into Lotus at the moment.

@adlrocha adlrocha closed this Jan 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.