joshstevens19 / simple-uniswap-sdk

Uniswap SDK which handles the routes automatically for you, changes in trade quotes reactive subscriptions, exposure to formatted easy to understand information, bringing back the best trade quotes automatically, generating transactions for you and much more.
MIT License
188 stars 95 forks source link

TokensFactoryPublic with customNetwork not working #41

Open niZmosis opened 2 years ago

niZmosis commented 2 years ago

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 commented 2 years ago

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.

joshstevens19 commented 2 years ago

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

niZmosis commented 2 years ago

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 commented 2 years ago

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.

joshstevens19 commented 2 years ago

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 commented 2 years ago

@LarryRyan0824

niZmosis commented 2 years ago

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

joshstevens19 commented 2 years ago

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?!

niZmosis commented 2 years ago

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.

joshstevens19 commented 2 years ago

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!

niZmosis commented 1 year ago

Fixed in PR #55