-
Notifications
You must be signed in to change notification settings - Fork 184
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
POS-15 Remove building Heimdall for each chain #738
Conversation
…ncli to master versions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
I forgot to edit README.md and to remove one line from .gitignore and makefile referencing heimdall-params.go |
LGTM! I will add also Arpit to review since he is the one with the most experience with Heimdall and also so that he can be on the loop. |
Codecov Report
@@ Coverage Diff @@
## master #738 +/- ##
=======================================
Coverage 72.88% 72.88%
=======================================
Files 49 49
Lines 3507 3507
=======================================
Hits 2556 2556
Misses 723 723
Partials 228 228 Continue to review full report at Codecov.
|
As commented over Slack, please change the target branch for the PR to v2 and we can merge it. |
a new Heimdall binary gets built for each chain using Makefile and including some config parameters at build time.
Build is done with:
https://github.com/maticnetwork/heimdall#usage
Heimdall runs this function to generate the heimdall_params.go:
https://github.com/maticnetwork/heimdall/blob/master/Makefile#L34
https://github.com/maticnetwork/heimdall/blob/master/helper/heimdall-params.template.go
It is only used to set this param for the client:
https://github.com/maticnetwork/heimdall/blob/master/helper/heimdall-params.template.go#L20
The goal is to move those parameters to Heimdall and use a flag:
$ heimdall --chain=[mumbai,mainnet,local]
to run the binary for each chain.
Internally, depending on the chain set the value of the NewSelectionAlgoHeight value.
Proposal:
There are multiple copying of chain toml files (during build and during init), maybe we should hard-code NewSelectionAlgoHeights and create files (or just one toml) during init phase