simple-uniswap-sdk icon indicating copy to clipboard operation
simple-uniswap-sdk copied to clipboard

TokensFactoryPublic with customNetwork not working

Open niZmosis opened this issue 3 years ago • 11 comments

I am attempting to get getAllowanceAndBalanceOfForContracts working with the TokensFactoryPublic for BSC. The constructor takes in a custom network but it doesn't seem to be taking the property as the factory logs out its custom network as undefined.

Code example: const tokensFactoryPublic = new TokensFactoryPublic({ ethereumProvider: provider, customNetwork: { nameNetwork: 'Binance Smart Chain Testnet', multicallContractAddress: '0x8F3273Fb89B075b1645095ABaC6ed17B2d4Bc576', nativeCurrency: { name: 'Binance Coin', symbol: 'tBNB', // BNB decimals: 18 }, nativeWrappedTokenInfo: { chainId: provider._network.chainId, contractAddress: '0xae13d989dac2f0debff460ac112a837c89baa7cd', decimals: 18, symbol: 'WBNB', name: 'Wrapped BNB' } } })

    console.log(tokensFactoryPublic)

Here is what it logs out: TokensFactoryPublic { _ethersProvider: EthersProvider { _providerContext: { ethereumProvider: [JsonRpcProvider], customNetwork: [Object] },
_ethersProvider: JsonRpcProvider { _isProvider: true, _events: [], _emitted: [Object], disableCcipRead: false, formatter: [Formatter], anyNetwork: false, _network: [Object], _maxInternalBlockNumber: -1024, _lastBlockNumber: -2, _maxFilterBlockRange: 10, _pollingInterval: 4000, _fastQueryDate: 0, connection: [Object], _nextId: 42 } }, _customNetwork: undefined, _cloneUniswapContractDetails: undefined, _multicall: CustomMulticall { _options: { ethersProvider: [JsonRpcProvider], tryAggregate: true, multicallCustomContractAddress: undefined }, ABI: [ [Object], [Object] ], _executionType: 'ethers' } }

niZmosis avatar Mar 15 '22 17:03 niZmosis

You can see in the _providerContext of the log that I gave it a custom network, but other things further down are undefined like the _custoNetwork and multicallCustomContractAddress.

niZmosis avatar Mar 27 '22 19:03 niZmosis

Sorry been super busy.. il look into this tomorrow for you and come back with an answer 👍

joshstevens19 avatar Mar 27 '22 20:03 joshstevens19

Here is my fix for it. For what ever reason the customNetwork does get through but instead of being under tokensFactoryObj._customNetwork, it get applied to tokensFactoryObj._ethersProvider._providerContext.customNetwork. So inside tokens.factory.js, I changed line 52. After this, the tokensFacrotyObj console log is fully filled out and my token balances are populated in my app.

function TokensFactory(_ethersProvider, _customNetwork, _cloneUniswapContractDetails) { var _a; this._ethersProvider = _ethersProvider; this._customNetwork = _customNetwork || _ethersProvider._providerContext.customNetwork; // Change this._cloneUniswapContractDetails = _cloneUniswapContractDetails; this._multicall = new CustomMulticall(this._ethersProvider.provider, (_a = this._customNetwork) === null || _a === void 0 ? void 0 : _a.multicallContractAddress); }

niZmosis avatar Apr 19 '22 00:04 niZmosis

So what I am seeing is the TokensFactoryPublic only exposes the first parameter of its super (TokensFactory). The first parameter is the EthereumProvider Interface which defines the provider and customNetwork. The TokensFactory is expecting its own customNetwork object which it never gets, since it is assumed it will be passed into the constructor as the second parameter, which the TokensFactoryPublic doesn't expose. This also means the _cloneUniswapContractDetails won't get set either. Maybe for TokensFactoryPublic we can set its constructor to (providerContext, _cloneUniswapContractDetails) and take the custom network from the providerContext within TokensFactory.

This explains why custom networks worked when configuring through the UniswapPairSettings for trades, as it uses TokensFactory itself, not TokensFactoryPublic.

niZmosis avatar May 11 '22 05:05 niZmosis

hey larry syncing back here what our your outstanding issues with uniswap sdk if you list them with examples il try to fix them all for you!

joshstevens19 avatar Jun 24 '22 18:06 joshstevens19

@LarryRyan0824

joshstevens19 avatar Jun 24 '22 18:06 joshstevens19

Hey, thanks for getting back to me. This thread is the last issue I have. I hope I explained what I found well enough for you to fix it in the comment above.

I dunno how easy it would be to expose price impact in the quote, but if it's not too much that'd be nice lol

One more thing on my mind which isn't a big deal at all. How we wrap the wrapped token to make it the native coin with _ETH, it'd be clearer if it was _Native as we deal with multiple chains in this library now.

As the other simple libraries are deprecated, it would be nice if the library had presets for the other main chains like BSC and Polygon. I have the custom networks setup for them, it just be a nice thing to have. But really this thread itself is my only problem, just getting into extras now lol.

@joshstevens19

niZmosis avatar Jun 24 '22 18:06 niZmosis

Ok @LarryRyan0824 going to schedule some time for me to investigate the above.. also yes I supported a lot of the other libs maybe some examples in here how to use custom networks with stuff like sushi swap and pancake etc would be good?!

joshstevens19 avatar Jun 24 '22 20:06 joshstevens19

Awesome, thanks man, yes I bet that would help out a lot for people. You ever think about moving the docs into a gitbook? If I ever get time I would love to contribute to all of this. React hooks wrapper is something I'd like to do for it, along with a base UI. Other things like token lists, even subgraphs.

niZmosis avatar Jun 24 '22 20:06 niZmosis

The library ended up growing a lot but I wouldn’t be against moving it to gitbook it’s just the time etc! I think GitHub readme would be ok for now due to the limited stuff the package does IMO. Would love you to make a package which uses this for react hooks etc would be very cool!

joshstevens19 avatar Jun 24 '22 20:06 joshstevens19

Fixed in PR #55

niZmosis avatar Nov 14 '22 02:11 niZmosis