Skip to content
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

TokensFactoryPublic with customNetwork not working #41

Open
niZmosis opened this issue Mar 15, 2022 · 11 comments
Open

TokensFactoryPublic with customNetwork not working #41

niZmosis opened this issue Mar 15, 2022 · 11 comments
Assignees

Comments

@niZmosis
Copy link

niZmosis commented Mar 15, 2022

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
Copy link
Author

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
Copy link
Owner

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

@joshstevens19 joshstevens19 self-assigned this Mar 27, 2022
@niZmosis
Copy link
Author

niZmosis commented Apr 19, 2022

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
Copy link
Author

niZmosis commented May 11, 2022

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
Copy link
Owner

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
Copy link
Owner

@LarryRyan0824

@niZmosis
Copy link
Author

niZmosis commented Jun 24, 2022

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
Copy link
Owner

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
Copy link
Author

niZmosis commented Jun 24, 2022

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
Copy link
Owner

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
Copy link
Author

Fixed in PR #55

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants