-
Notifications
You must be signed in to change notification settings - Fork 274
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(sdk-coin-bsc): rename bsc to bnb #3701
Conversation
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.
All these commits will need to be marked as breaking changes in the commit footer. Commit descriptions should also outline the breaking change for external consumers.
8b68cbf
to
7ec3389
Compare
We want to be wary of a possible HSM change as well. |
Has there been consideration in not touching this module until it is fully deprecated and spinning up a new bnb module? |
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.
Think we can keep the chain name as Binance smart chain itself, Not BNB smart chain, Other than that things looks good, Adding do not merge for this untill we made associated changes in all other Microservices. Think we have a lot of places to update inorder to have this updated everywhere and also we can consider @mmcshinsky-bitgo suggestion so that this becomes non-breaking
'Binance Smart Chain', | ||
Networks.main.bsc, | ||
'bnb', | ||
'BNB Smart Chain', |
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.
Why change Binance to BNB?
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.
Oh I think BSC is trying to distance itself from Binance exchange so they want to brand BSC (the chain) as BNB Smart Chain. That's what I read at least.
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.
The Binance Chain, where staking and voting occur (BNB Chain Governance), has changed its identity to BNB Beacon Chain under the rebranded BNB Chain umbrella. The Binance Smart Chain, which supports several blockchains and is compatible with the EVM, is known simply as BNB Smart Chain, still abbreviated as BSC.
'Testnet Binance Smart Chain', | ||
Networks.test.bsc, | ||
'tbnb', | ||
'Testnet BNB Smart Chain', |
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 think we could keep Binance as is ?
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.
Flushing from review Q, please re-request review once this is ready.
8307d3f
to
fe0de21
Compare
I think this is good route to take --- I'll follow up with team. |
Ticket: EA-709
This is a breaking change for services that rely on SDK BSC constants like WP. Will verify corresponding WP changes with alpha release.