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

chore: upgrade multibuild #113

Open
wants to merge 15 commits into
base: main
Choose a base branch
from
Open

Conversation

RafilxTenfen
Copy link
Contributor

@RafilxTenfen RafilxTenfen commented Oct 1, 2024

  • Slipt the upgrade data into two
    • testnet app/upgrades/v1/testnet
    • mainnet app/upgrades/v1/mainnet
  • Add new build tag mainnet or testnet that adds the upgrade handler with the expected data
  • By default make build creates a binary with an upgrade plan that contains mainnet data, if make build-testnet is run it adds the testnet build flag and adds the upgrade plan that contains testnet data

@RafilxTenfen RafilxTenfen self-assigned this Oct 1, 2024
@RafilxTenfen RafilxTenfen marked this pull request as ready for review October 2, 2024 16:41
@RafilxTenfen RafilxTenfen requested a review from a team as a code owner October 2, 2024 16:41
@RafilxTenfen RafilxTenfen requested review from gitferry, samricotta, KonradStaniec and a team and removed request for a team October 2, 2024 16:41
Copy link
Contributor Author

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 {
Copy link
Contributor Author

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 {
Copy link
Contributor Author

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"
Copy link
Contributor Author

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{
Copy link
Contributor Author

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{
Copy link
Contributor Author

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

Copy link
Member

@SebastianElvis SebastianElvis left a 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
Copy link
Member

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 🤔

Copy link
Contributor Author

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

babylon/Makefile

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

app/upgrades/types.go Outdated Show resolved Hide resolved
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.

Mechanism of switching different upgrade data between different networks (mainnet/testnet/devnet)
2 participants