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

feat: added Optimism and Arbitrum support (updated constants.py) #186

Merged
merged 2 commits into from
Oct 25, 2021

Conversation

liquid-8
Copy link
Member

Closes #185
Closes #183
Closes #176

@romanbsd
Copy link
Contributor

Also 421611 => "arbitrum_testnet"

@romanbsd
Copy link
Contributor

BTW, I'm using uniswap-python on arbitrum, and I had to monkey patch _get_tx_params and remove the "gas" parameter, otherwise I was receiving "insufficient funds for gas". Perhaps it's something specific to my setup, but I thought that it might be useful for others if they also face such problem.

@liquid-8
Copy link
Member Author

I've checked some uni swaps on arbitrum and it turns out txn requires gas spendings ~1m but _get_tx_param has hardcoded 250k limit. So I'm quite sure it isn't about your setup.

@romanbsd
Copy link
Contributor

Yes, I can testify: 1,078,834
I wasn't sure that it's not something specific to my case. I'm curious why it's different from L1 gas usage.

@ErikBjare
Copy link
Member

ErikBjare commented Oct 25, 2021

BTW, I'm using uniswap-python on arbitrum, and I had to monkey patch _get_tx_params and remove the "gas" parameter, otherwise I was receiving "insufficient funds for gas".

Perhaps the gas parameter could be removed entirely from _get_tx_params? Perhaps we should use estimate_gas instead?

I wasn't sure that it's not something specific to my case. I'm curious why it's different from L1 gas usage.

Could it be because each LP position is smaller, and this impacts gas efficiency? (just guessing) Edit: Nevermind, answers here https://developer.offchainlabs.com/docs/arbgas

@ErikBjare
Copy link
Member

Merging this for now, we'll deal with any issues in future PRs.

@ErikBjare ErikBjare changed the title Update constants.py; added Optimism and Arbitrum support feat: added Optimism and Arbitrum support (updated constants.py) Oct 25, 2021
@ErikBjare ErikBjare merged commit 754fbc8 into uniswap-python:master Oct 25, 2021
@romanbsd
Copy link
Contributor

Uniswap3 suggest doing 1.2 * estimateGas. I can make a PR if you want.

@ErikBjare
Copy link
Member

@romanbsd That would be appreciated :)

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

Successfully merging this pull request may close these issues.

Add support for Arbitrum One
3 participants