-
Notifications
You must be signed in to change notification settings - Fork 52
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
chore: upgrade multibuild #113
Conversation
…as BABYLON_BUILD_OPTIONS=testnet
…upgrade-multibuild
…upgrade-multibuild
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.
At the moment, data files from the mainnet and testnet are the same but just to simulate/test, when we get close to launch, the testnet and mainnet will have different data files
|
||
func TestHardCodedBtcStakingParamsAreValid(t *testing.T) { | ||
bbnApp := app.NewTmpBabylonApp() | ||
for _, upgradeData := range UpgradeV1Data { |
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.
each unit test now check both testnet and mainnet data files
AddressReceiver string `json:"address_receiver"` | ||
Amount int64 `json:"amount"` | ||
} `json:"token_distribution"` | ||
func CreateUpgrade(upgradeDataStr UpgradeDataString) upgrades.Upgrade { |
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.
avoid having a var for each upgrade, by accepting the upgrade data as parameter
@@ -9,7 +9,7 @@ babylond: babylond-rmi | |||
|
|||
babylond-e2e: | |||
docker build --tag babylonlabs-io/babylond -f babylond/Dockerfile ${BABYLON_FULL_PATH} \ | |||
--build-arg BUILD_TAGS="e2e" | |||
--build-arg BABYLON_BUILD_OPTIONS="testnet" |
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.
Fixed to use correct build arg, BUILD_TAGS
should only be set by the makefile
// init is used to include v1 upgrade testnet data | ||
// it is also used for e2e testing | ||
func init() { | ||
Upgrades = []upgrades.Upgrade{v1.CreateUpgrade(v1.UpgradeDataString{ |
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.
only place beside tests that imports testnet and is included only if there is a testnet
in build tags
|
||
// init is used to include v1 upgrade for mainnet data | ||
func init() { | ||
Upgrades = []upgrades.Upgrade{v1.CreateUpgrade(v1.UpgradeDataString{ |
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.
only place beside tests that imports mainnet and is included only if there is a mainnet
in build tags
Makefile sets by default this build tag
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.
I have less context here, so only some (non-blocking) general questions
@@ -0,0 +1,20 @@ | |||
//go:build mainnet |
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.
I remember Go allows to build with multiple tags e.g., -tags "mainnet,testnet"
. Wondering what will happen in this case 🤔
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.
good question, in both cases I am replacing the upgrades var from app.go with a new upgrade
So, the question is which one ? 😅
One other point is that in the makefile or it includes mainnet or testnet to the build tags
Lines 82 to 87 in aea173f
# Handles the inclusion of upgrade in binary | |
ifeq (testnet,$(findstring testnet,$(BABYLON_BUILD_OPTIONS))) | |
BUILD_TAGS += testnet | |
else | |
BUILD_TAGS += mainnet | |
endif |
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.
Looks great!
app/upgrades/v1/testnet
app/upgrades/v1/mainnet
mainnet
ortestnet
that adds the upgrade handler with the expected datamake build
creates a binary with an upgrade plan that contains mainnet data, ifmake build-testnet
is run it adds thetestnet
build flag and adds the upgrade plan that contains testnet data