feat: deprecate Network, use explicit passphrase (2)

[Akuukis]

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!

[Akuukis]

@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.

[Akuukis]

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

[abuiles]

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

[MisterTicot]

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]

@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]

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

[abuiles]

@mkaliberda can you elaborate please?

[MisterTicot]

@Akuukis This

[marcinx]

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]

@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]

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

[marcinx]

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]

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]

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]

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.

[bantustic]

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.