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 blst #3644

Closed
wants to merge 4 commits into from
Closed

add blst #3644

wants to merge 4 commits into from

Conversation

boler99
Copy link

@boler99 boler99 commented Apr 8, 2021

Summary

  • add blst
  • use herumi/bls-eth-go-binary instead of oudated forks
  • no more bls wrappers around

Issue

harmony-one/bounties/issues/8

Test

Operational Checklist

  1. Does this PR introduce backward-incompatible changes to the on-disk data structure and/or the over-the-wire protocol?. (If no, skip to question 8.)

No

  1. Does this PR introduce backward-incompatible changes NOT related to on-disk data structure and/or over-the-wire protocol? (If no, continue to question 11.)

No Probably, see comments below

  1. Does this PR introduce significant changes to the operational requirements of the node software, such as >20% increase in CPU, memory, and/or disk usage?

No

TODO

Build scripts need to be updated

@LeoHChen
Copy link
Contributor

LeoHChen commented Apr 9, 2021

@boler99 the build failed? did you pass the local build?

@boler99
Copy link
Author

boler99 commented Apr 9, 2021

@LeoHChen I'm afraid build scripts are too confusing for me. I believe you could sort them out better according to library changes.

@boler99 boler99 marked this pull request as draft April 9, 2021 18:41
label pubkey cache against lib

bring back original test keys

introduce bigendian serialization but with no luck
@boler99
Copy link
Author

boler99 commented Apr 9, 2021

This PR wants to use bls libraries that follows current standarts and hasitates to customize them. These libraries are namely blst and herumi-eth.

Either current serialization of keys or worse bls protocol in use for example it might be generator of the group is not inline with the standart.

We have tried to figure out what's different and make it backwards compatible but without a luck. It is most probably the generators, see links above.

So tests below which are related to hardcoded keys and files are failing.

TestValidateConsensusKeysForSameShard
ExampleLoadKeys
TestPassDecrypter_decryptFile
TestBlsDirLoader
TestMultiKeyLoader_loadKeys

@LeoHChen
Copy link
Contributor

This PR wants to use bls libraries that follows current standarts and hasitates to customize them. These libraries are namely blst and herumi-eth.

Either current serialization of keys or worse bls protocol in use for example it might be generator of the group is not inline with the standart.

We have tried to figure out what's different and make it backwards compatible but without a luck. It is most probably the generators, see links above.

So tests below which are related to hardcoded keys and files are failing.

TestValidateConsensusKeysForSameShard
ExampleLoadKeys
TestPassDecrypter_decryptFile
TestBlsDirLoader
TestMultiKeyLoader_loadKeys

@rlan35 , would you please take a look at the serialization or the generator issue mentioned above?

@AlexiaChen
Copy link
Contributor

AlexiaChen commented Apr 19, 2021

@boler99 Fetch your pr, build failed locolly, error message shown as follows:

../../../../pkg/mod/github.com/herumi/bls-eth-go-binary@v0.0.0-20210407105559-9588dcfc7de7/bls/bls.go:36:24: could not determine kind of name for C.BLS_ETH_MODE_DRAFT_05
../../../../pkg/mod/github.com/herumi/bls-eth-go-binary@v0.0.0-20210407105559-9588dcfc7de7/bls/bls.go:37:24: could not determine kind of name for C.BLS_ETH_MODE_DRAFT_06
../../../../pkg/mod/github.com/herumi/bls-eth-go-binary@v0.0.0-20210407105559-9588dcfc7de7/bls/bls.go:38:24: could not determine kind of name for C.BLS_ETH_MODE_DRAFT_07
../../../../pkg/mod/github.com/herumi/bls-eth-go-binary@v0.0.0-20210407105559-9588dcfc7de7/bls/bls.go:39:23: could not determine kind of name for C.BLS_ETH_MODE_LATEST
../../../../pkg/mod/github.com/herumi/bls-eth-go-binary@v0.0.0-20210407105559-9588dcfc7de7/bls/bls.go:35:20: could not determine kind of name for C.BLS_ETH_MODE_OLD
../../../../pkg/mod/github.com/herumi/bls-eth-go-binary@v0.0.0-20210407105559-9588dcfc7de7/bls/bls.go:839:2: could not determine kind of name for C.blsAggregateSignature
../../../../pkg/mod/github.com/herumi/bls-eth-go-binary@v0.0.0-20210407105559-9588dcfc7de7/bls/bls.go:971:9: could not determine kind of name for C.blsAggregateVerifyNoCheck
../../../../pkg/mod/github.com/herumi/bls-eth-go-binary@v0.0.0-20210407105559-9588dcfc7de7/bls/bls.go:848:9: could not determine kind of name for C.blsFastAggregateVerify
../../../../pkg/mod/github.com/herumi/bls-eth-go-binary@v0.0.0-20210407105559-9588dcfc7de7/bls/bls.go:818:9: could not determine kind of name for C.blsMultiVerifyFinal
../../../../pkg/mod/github.com/herumi/bls-eth-go-binary@v0.0.0-20210407105559-9588dcfc7de7/bls/bls.go:781:4: could not determine kind of name for C.blsMultiVerifySub
../../../../pkg/mod/github.com/herumi/bls-eth-go-binary@v0.0.0-20210407105559-9588dcfc7de7/bls/bls.go:912:9: could not determine kind of name for C.blsPublicKeyDeserializeUncompressed
../../../../pkg/mod/github.com/herumi/bls-eth-go-binary@v0.0.0-20210407105559-9588dcfc7de7/bls/bls.go:457:9: could not determine kind of name for C.blsPublicKeyIsZero
../../../../pkg/mod/github.com/herumi/bls-eth-go-binary@v0.0.0-20210407105559-9588dcfc7de7/bls/bls.go:891:7: could not determine kind of name for C.blsPublicKeySerializeUncompressed
../../../../pkg/mod/github.com/herumi/bls-eth-go-binary@v0.0.0-20210407105559-9588dcfc7de7/bls/bls.go:299:9: could not determine kind of name for C.blsSecretKeyIsZero
../../../../pkg/mod/github.com/herumi/bls-eth-go-binary@v0.0.0-20210407105559-9588dcfc7de7/bls/bls.go:934:12: could not determine kind of name for C.blsSetETHmode
../../../../pkg/mod/github.com/herumi/bls-eth-go-binary@v0.0.0-20210407105559-9588dcfc7de7/bls/bls.go:922:9: could not determine kind of name for C.blsSignatureDeserializeUncompressed
../../../../pkg/mod/github.com/herumi/bls-eth-go-binary@v0.0.0-20210407105559-9588dcfc7de7/bls/bls.go:560:9: could not determine kind of name for C.blsSignatureIsZero
../../../../pkg/mod/github.com/herumi/bls-eth-go-binary@v0.0.0-20210407105559-9588dcfc7de7/bls/bls.go:902:7: could not determine kind of name for C.blsSignatureSerializeUncompressed

Question:

Why use herumi/bls-eth-go-binary intead of harmonry forked herumi bls? it works well even outdated.

I know the reason why herumi/bls-eth-go-binary intead of harmonry forked herumi bls. In this way, it's convenient for you to unify the different interfaces of the two libraries. The latest herumi library has added a lot of APIs, because I replaced it with the obsolete herumi and found a lot of undefined problems when build.

I can't compile normally by simply changing a few pieces of code. Is there such an error in your local environment? What should I do?

I just run go test and found that the serialization or the generator issue mentioned above.

@boler99
Copy link
Author

boler99 commented Apr 19, 2021

@AlexiaChen

Why use herumi/bls-eth-go-binary intead of harmonry forked herumi bls? it works well even outdated.

Simply altering blst generators (public key base point) also may not work since hash to curve method has changed several times in result of the standardization effort.

I can't compile normally by simply changing a few pieces of code. Is there such an error in your local environment? What should I do?

I've managed to pass these errors simply commenting out all lines related with two harmony bls lib in build scripts. I think it happens because there are two C library with same interface one is older one and the other is bls-eth-go-binary.

@AlexiaChen
Copy link
Contributor

AlexiaChen commented Apr 19, 2021

Why use herumi/bls-eth-go-binary intead of harmonry forked herumi bls? it works well even outdated.

Simply altering blst generators (public key base point) also may not work since hash to curve method has changed several times in result of the standardization effort.

I can't compile normally by simply changing a few pieces of code. Is there such an error in your local environment? What should I do?

I've managed to pass these errors simply commenting out all lines related with two harmony bls lib in build scripts. I think it happens because there are two C library with same interface one is older one and the other is bls-eth-go-binary.

Coud you Fix it in this PR and pass building in the future (not locally)? . Yeah, the bls-eth-go-binary alseo included submodules mcl and bls. maybe could lead this error happens. I will follow this way, try to fix it locally , If I have any problems later, I'll ask you again @boler99

@AlexiaChen
Copy link
Contributor

AlexiaChen commented Apr 19, 2021

@boler99 i just pull your update, it build success on linux(ubuntu 18.04), but use ldd to loop up some dependencies, denpencies related bls hash dispeared, Even if you use a new BLS library, you will still depends on MCL and the old BLS so library? bls-eth-go-binary use statci library default? I just read readme of the library repo and author explained the library only support static lib, so I could not find any dynamical library related BLS?

Yes I just confirm it indeed use static library with go build exmaple/sample.go on the repo, it run well without so library. Thanks.

@gupadhyaya
Copy link
Contributor

@boler99 the current generator (base point) used in our bls & mcl are different from ETH2.0. We see that both bls and mcl have added support for this: https://github.com/herumi/bls/blob/master/src/bls_c_impl.hpp#L94. we are currently exploring the proper update to bls and mcl forks to make it consistent.

@AlexiaChen
Copy link
Contributor

AlexiaChen commented Apr 20, 2021

@boler99
bootnode panic crash with nil pointer , because bootnode HostConfig Self.ConsensusPubKey is nil

	selfPeer := p2p.Peer{IP: *ip, Port: *port}
	host, err := p2p.NewHost(p2p.HostConfig{
		Self:          &selfPeer,
		BLSKey:        privKey,
		BootNodes:     nil, // Boot nodes have no boot nodes :) Will be connected when other nodes joined
		DataStoreFile: &dataStorePath,
	})
	utils.Logger().Info().
		Str("self", net.JoinHostPort(self.IP, self.Port)).
		Interface("PeerID", self.PeerID).
		Str("PubKey", self.ConsensusPubKey.ToHex()). // crash here
		Msg("libp2p host ready")

before your modification, Pubkey (not interface type) method is SerializeToHexStr() not ToHex(), the function impl checks obj pointer is valid, so not crash:

func (pub *PublicKey) SerializeToHexStr() string {
	if pub == nil {
		return ""
	}
	return hex.EncodeToString(pub.Serialize())
}

So you need to check PublicKey (interface type ) if it is nil now

@AlexiaChen
Copy link
Contributor

AlexiaChen commented Apr 20, 2021

@boler99

This issue #3644 (comment) has been solved locally by me mentioned above, but a new problem comes out. When I start the local network test, I report an error in the function of setupLegacyNodeAccount: "error cannot find your BLS key in the genesis / FN tables: XXX". After a period of debugging, I found that the corresponding BLS public key could not be found in the local network instances from v0 to V3 in the findAccountsByPubKeys function, and the corresponding public key should be hard coded to localnet.go For example, LocalHarmonyAccounts, LocalFnAccountsV1, etc. This problem should be caused by the different base points of elliptic curve in the new BLS library, which leads to the different public keys corresponding to the private keys? pubkey = secretKey*G

related functions as follows:

func setupConsensusKeys(hc harmonyConfig, config *nodeconfig.ConfigType) multibls.PublicKeys {
	onceLoadBLSKey.Do(func() {
		var err error
		multiBLSPriKey, err = loadBLSKeys(hc.BLSKeys)
		if err != nil {
			fmt.Fprintf(os.Stderr, "ERROR when loading bls key: %v\n", err)
			os.Exit(100)
		}
	})

	config.ConsensusPriKey = multiBLSPriKey
        // generated different pubkey from same secret key
	return multiBLSPriKey.GetPublicKeys()
}
func (sc instance) FindAccount(blsPubKey string) (bool, *genesis.DeployAccount) {
	for i, item := range sc.hmyAccounts {
		if item.BLSPublicKey == blsPubKey {
			item.ShardID = uint32(i) % sc.numShards
			return uint32(i) < sc.numShards, &item
		}
	}
	for i, item := range sc.fnAccounts {
		if item.BLSPublicKey == blsPubKey {
			item.ShardID = uint32(i) % sc.numShards
			return false, &item
		}
	}
	return false, nil
}

would you please please take a look at new problem @LeoHChen @rlan35 @gupadhyaya

@gupadhyaya
Copy link
Contributor

@boler99

This issue #3644 (comment) has been solved locally by me mentioned above, but a new problem comes out. When I start the local network test, I report an error in the function of setupLegacyNodeAccount: "error cannot find your BLS key in the genesis / FN tables: XXX". After a period of debugging, I found that the corresponding BLS public key could not be found in the local network instances from v0 to V3 in the findAccountsByPubKeys function, and the corresponding public key should be hard coded to localnet.go For example, LocalHarmonyAccounts, LocalFnAccountsV1, etc. This problem should be caused by the different base points of elliptic curve in the new BLS library, which leads to the different public keys corresponding to the private keys? pubkey = secretKey*G

related functions as follows:

func setupConsensusKeys(hc harmonyConfig, config *nodeconfig.ConfigType) multibls.PublicKeys {
	onceLoadBLSKey.Do(func() {
		var err error
		multiBLSPriKey, err = loadBLSKeys(hc.BLSKeys)
		if err != nil {
			fmt.Fprintf(os.Stderr, "ERROR when loading bls key: %v\n", err)
			os.Exit(100)
		}
	})

	config.ConsensusPriKey = multiBLSPriKey
        // generated different pubkey from same secret key
	return multiBLSPriKey.GetPublicKeys()
}
func (sc instance) FindAccount(blsPubKey string) (bool, *genesis.DeployAccount) {
	for i, item := range sc.hmyAccounts {
		if item.BLSPublicKey == blsPubKey {
			item.ShardID = uint32(i) % sc.numShards
			return uint32(i) < sc.numShards, &item
		}
	}
	for i, item := range sc.fnAccounts {
		if item.BLSPublicKey == blsPubKey {
			item.ShardID = uint32(i) % sc.numShards
			return false, &item
		}
	}
	return false, nil
}

would you please please take a look at new problem @LeoHChen @rlan35 @gupadhyaya

thanks @AlexiaChen for your investigation. Yes, the core of the problem requires changing our base points to be complaint with others. This means that we have to redo all the private keys (local, testnet, mainnet) to be able to successfully extract the public keys and verify the signatures. you can pause this investigation for now. we are working on finding the best way to proceed. will update here once the decision is made.

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.

None yet

6 participants