feat: deprecate Network, use explicit passphrase (2) #207

Akuukis · 2019-07-19T07:34:38Z

Deprecate global singleton for Network. The following classes and methods take an optional network passphrase, and issue a warning if it is not passed:

`Keypair.master`

Keypair.master(Networks.TESTNET)

constructor for `Transaction`

const xenv = new xdr.TransactionEnvelope({ tx: xtx });
new Transaction(xenv, Networks.TESTNET);

constructor for `TransactionBuilder` and method `TransactionBuilder.setNetworkPassphrase`

const transaction = new StellarSdk.TransactionBuilder(account, {
  fee: StellarSdk.BASE_FEE,
  networkPassphrase: Networks.TESTNET
})

The Network class will be removed on the 2.0 release.

Fixes #112.

abuiles

@Akuukis Looking good!

Besides the inline comments, could you do the following:

Add a new section to the changelog called ### Unreleased and document this change
Update the example here to use the new syntax https://github.com/Akuukis/js-stellar-base/blob/explicit-network-2/src/transaction.js#L106

We also need to update the docs to make sure they reflect the new syntax, but it doesn't block this PR.

Thanks!

src/transaction_builder.js

Akuukis · 2019-07-26T12:07:28Z

@abuiles Done! Anything else?

abuiles

@Akuukis looking good! One last change and I think we'll be good :). Let's avoid the breaking change for now and use a getter approach.

docs/reference/building-transactions.md

src/transaction.js

docs/reference/building-transactions.md

CHANGELOG.md

Akuukis · 2019-07-31T09:19:06Z

@abuiles Review implemented, and all tests files reverted too (so no breaking changes noticed). Anything else?

abuiles · 2019-07-31T15:16:26Z

@Akuukis Thanks! I'll release this later today :)

src/network.js

MisterTicot · 2019-08-10T13:46:37Z

Hi guys, I just landed here after running a

StellarSdk.Network.useTestNetwork()

Global class Network is deprecated. Please pass explicit argument instead, e.g. new Transaction(envelope, Networks.PUBLIC) (see https://git.io/fj9fG for more info).

The only reason I called that function was to make it happy, though:

Error: No network selected. Use Network.use, Network.usePublicNetwork or Network.useTestNetwork helper methods to select network.

Akuukis · 2019-08-10T18:04:56Z

@MisterTicot Hi! Hmm, I'm ready to take a look at it. Can you please share the link to your code or elaborate more on the context of error? Or, even better, extract a small snippet where it fails?

mkaliberda · 2019-08-13T11:30:28Z

I use stellar-sdk.js when You apdated It, stellar-sdk does not work with old version!

abuiles · 2019-08-13T15:13:05Z

@mkaliberda can you elaborate please?

MisterTicot · 2019-08-15T10:30:47Z

@Akuukis This

marcinx · 2019-09-23T18:55:13Z

Recently updated to newer versions and now see warnings in our logs: Sep 23 18:48:18 localhost stardust: Global class 'Network' is deprecated. Please pass explicit argument instead, e.g. 'new Transaction(envelope, Networks.PUBLIC)' (see https://git.io/fj9fG for more info).

This appears to be triggered by StellarSdk.Network.usePublicNetwork(), and StellarSdk.Network.useTestNetwork().

I don't understand the motivation behind this change by the way. The error message suggests that we now need to pass the network for every single transaction issued in our application? I am a little bit confused about the warning and the suggested solution (passing the network to tx constructor every time).

Looking forward to some input on this @abuiles Thanks!

morleyzhi · 2019-09-23T20:35:42Z

@marcinx The original issue, #112, lays out the reasoning behind the change. In my personal opinion, having ported some code to the new way of setting networks, the change isn't that bad and accommodates more use cases like the one in the linked issue, at the expense of a few keystrokes.

msfeldstein · 2019-09-23T20:37:29Z

Ideally you'd use an environment variable so you can easily change networks, including between testnet and mainnet deployments

marcinx · 2019-09-23T20:59:25Z

Okay. Thanks for the follow ups.

So basically we can scrap StellarSdk.Network.usePublicNetwork() and forget about the singleton.

What network is selected by default if we don't pass a passphrase to TransactonBuilder? It looks like the passphrase is being set to null if no opts.networkPassphrase is given: this.networkPassphrase = opts.networkPassphrase || null; What happens when the passphrase is null?

We currently don't work with Testnet. Do we need to specifically add Networks.PUBLIC (or env var, or some other way, etc.) to each transaction builder call or will it automatically select PUBLIC if no such opt is provided?

Edit: Looks like it will simply throw an error down the road if the network is not set in the singleton. When support for the singleton is removed in the future the transactions will simply fail if no network is explicitly set in the opts. Is that intended?

Wouldn't it be better to default the passPhrase to Networks.PUBLIC (or testnet) instead of null if it is not provided in the opts for transaction builder? I.e. this.networkPassphrase = opts.networkPassphrase || Networks.PUBLIC. ?

morleyzhi · 2019-09-23T21:08:09Z

Yes, I would specifically pass Networks.PUBLIC in places that expect a network passphrase.

When you don't pass in a passphrase, it will fall back to the current Network.use*() setting:

js-stellar-base/src/transaction.js

Line 70 in 64785eb

console.warn(

Ostensibly when we remove Network.use*(), we'll throw an error wherever a network passphrase is expected but not supplied.

msfeldstein · 2019-09-23T21:08:31Z

we definitely don't want to default things to Networks.PUBLIC, if anything we'd default to TESTNET, but its better to have stuff like this just fail so it can be fixed properly instead of continuing to work in an undefined way

marcinx · 2019-09-23T21:13:16Z

Okay, thanks for the super fast replies! Figured it all out now. I'm definitely glad I subscribed to js-stellar-base so I can keep track of these changes more continuously.

StellarSdk.Network.useTestNetwork() was deprecated by Networks.TESTNET constant, so this should be updated in order to be consistent with this update: stellar/js-stellar-base#207

Per deprecation warning message, which links to here: stellar/js-stellar-base#207

bantustic · 2021-10-03T13:40:40Z

For those using the sdk to connect with stellarSdk.Network.useTestNetwork() did not work for me and says deprecated on top of the page. I have only managed to get it to work with StellarSdk.Networks.TESTNET. best of luck.

This was referenced Jul 19, 2019

Network is a global singleton #112

Closed

feat: deprecate Network, use explicit passphrase (1) #206

Closed

abuiles added the ready for review label Jul 25, 2019

abuiles suggested changes Jul 25, 2019

View reviewed changes

src/transaction_builder.js Outdated Show resolved Hide resolved

abuiles suggested changes Jul 29, 2019

View reviewed changes

docs/reference/building-transactions.md Outdated Show resolved Hide resolved

src/transaction.js Outdated Show resolved Hide resolved

docs/reference/building-transactions.md Outdated Show resolved Hide resolved

abuiles reviewed Jul 29, 2019

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

Akuukis and others added 6 commits July 31, 2019 11:47

feat: deprecate Network, use explicit passphrase 2

5fee18f

test: adjust existing tests

44f05f3

fix(txb): typo

ba7dcc0

chore(docs): changelog for unreleased changes

2fe26ee

docs(transaction): use explicit networkPassphrase

bf1e47b

syle(docs): align code

262aec1

Akuukis force-pushed the explicit-network-2 branch from c648cc1 to 262aec1 Compare July 31, 2019 08:48

fix(transaction): solve breaking change

f577e85

abuiles approved these changes Jul 31, 2019

View reviewed changes

abuiles suggested changes Jul 31, 2019

View reviewed changes

src/network.js Show resolved Hide resolved

chore(*): deprecation warning links to PR

08c5d07

abuiles approved these changes Aug 1, 2019

View reviewed changes

abuiles merged commit 46c4349 into stellar:master Aug 1, 2019

abuiles mentioned this pull request Aug 5, 2019

Add types for new networkPassphrase argument. Fix #237. #238

Merged

Akuukis deleted the explicit-network-2 branch August 10, 2019 17:59

MisterTicot mentioned this pull request Aug 15, 2019

Fix no-network warnings #248

Merged

$@fracek$ fracek mentioned this pull request Sep 22, 2019

Deprecate/Remove Network singleton elucidsoft/dotnet-stellar-sdk#197

Closed

ernestognw mentioned this pull request Jan 11, 2020

Update transactions.md stellar-deprecated/docs#545

Merged

grempe added a commit to grempe/js-stellar-sdk that referenced this pull request Apr 28, 2020

Fix deprecation warning when building a transaction

d4a3275

Per deprecation warning message, which links to here: stellar/js-stellar-base#207

grempe mentioned this pull request Apr 28, 2020

Fix deprecation warning when building a transaction stellar/js-stellar-sdk#537

Merged

abuiles pushed a commit to stellar/js-stellar-sdk that referenced this pull request Apr 28, 2020

Fix deprecation warning when building a transaction (#537)

51bb5e2

Per deprecation warning message, which links to here: stellar/js-stellar-base#207

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: deprecate Network, use explicit passphrase (2) #207

feat: deprecate Network, use explicit passphrase (2) #207

Akuukis commented Jul 19, 2019 •

edited by abuiles

abuiles left a comment

Akuukis commented Jul 26, 2019

abuiles left a comment

Akuukis commented Jul 31, 2019 •

edited

abuiles commented Jul 31, 2019

MisterTicot commented Aug 10, 2019

Akuukis commented Aug 10, 2019

mkaliberda commented Aug 13, 2019 •

edited

abuiles commented Aug 13, 2019

MisterTicot commented Aug 15, 2019

marcinx commented Sep 23, 2019 •

edited

morleyzhi commented Sep 23, 2019

msfeldstein commented Sep 23, 2019

marcinx commented Sep 23, 2019 •

edited

morleyzhi commented Sep 23, 2019

msfeldstein commented Sep 23, 2019

marcinx commented Sep 23, 2019

bantustic commented Oct 3, 2021

feat: deprecate Network, use explicit passphrase (2) #207

feat: deprecate Network, use explicit passphrase (2) #207

Conversation

Akuukis commented Jul 19, 2019 • edited by abuiles

Keypair.master

constructor for Transaction

constructor for TransactionBuilder and method TransactionBuilder.setNetworkPassphrase

abuiles left a comment

Choose a reason for hiding this comment

Akuukis commented Jul 26, 2019

abuiles left a comment

Choose a reason for hiding this comment

Akuukis commented Jul 31, 2019 • edited

abuiles commented Jul 31, 2019

MisterTicot commented Aug 10, 2019

Akuukis commented Aug 10, 2019

mkaliberda commented Aug 13, 2019 • edited

abuiles commented Aug 13, 2019

MisterTicot commented Aug 15, 2019

marcinx commented Sep 23, 2019 • edited

morleyzhi commented Sep 23, 2019

msfeldstein commented Sep 23, 2019

marcinx commented Sep 23, 2019 • edited

morleyzhi commented Sep 23, 2019

msfeldstein commented Sep 23, 2019

marcinx commented Sep 23, 2019

bantustic commented Oct 3, 2021

Akuukis commented Jul 19, 2019 •

edited by abuiles

`Keypair.master`

constructor for `Transaction`

constructor for `TransactionBuilder` and method `TransactionBuilder.setNetworkPassphrase`

Akuukis commented Jul 31, 2019 •

edited

mkaliberda commented Aug 13, 2019 •

edited

marcinx commented Sep 23, 2019 •

edited

marcinx commented Sep 23, 2019 •

edited