Feat/single gateway balance allowance

[isreallee82]

Before submitting this PR, please make sure:

• [ ] Your code builds clean without any errors or warnings
• [ ] You are using approved title ("feat/", "fix/", "docs/", "refactor/")

A description of the changes proposed in the pull request: since there is a potential proposed feature for single balance display #3975.

Addittion of a new gateway balance command that reports balances for single connected wallets. Reafctor of all the necessary function to make the command work. Fix other related bugs.

Tests performed by the developer:

Added few exchanges tor the client pangolin_avalanche_avalanche ✅ uniswarp_ethereum_mainet ✅ uniswap_ethereum_arbitrum_one ✅ others pending

Tips for QA testing:

[isreallee82]

@fengtality @rapcmia @nikspz Would it be preferable for the overall gateway balance to include allowances, or should I maintain its current state of separate displays for each balance exchange, prioritizing a faster user experience?

[nikspz]

hi @isreallee82 I think https://github.com/hummingbot/hummingbot/issues/3975 Enhancement was requested in 2021 for CEX connectors like binance gate_io etc. to check separate balance (like balance binance, balance gate_io etc.). That was not a bounty, just a enhancement request. Will provide you updates later regarding gateway allowances implementation

[isreallee82]

sure I know clearly @nikspz

[isreallee82]

also is it right to create a pr for an enhancement as I did @nikspz?

[nikspz]

@isreallee82 Yes sure it's totally okay. Just make sure to discuss implementation before submitting to prevent not priority changes. Thank you for trying to improve the Client, really appreciate it

[isreallee82]

alright will note that @nikspz

[fengtality]

@isreallee82 Thanks for this contribution. I think adding allowances to the gateway balance command makes sense. However, I think some changes are needed to make this work properly for users.

• Adding the allowances column to gateway balance is preferable, so that the user can see the allowances for each exchange.

• I like your addition of the flag to the gateway balance command, since it should speed up operations for users who don't need balances for all their networks. However, I think it should be structured as a --chain flag with autocomplete for the chain_network, not the exchange_chain_network, like

gateway balance --chain ethereum_mainnet

• Currently, the autocomplete for commands that expect chain_network still shows exchange_chain_network. Change this to chain_network:

• Clean the gateway connect command so that:
  • the 2nd column is titled Chain_Network
  • the 2nd column data is formatted like ethereum_mainnet

[isreallee82]

Thank you for your valuable input and suggestions, @fengtality. i'll promptly implement the suggested modifications and push the updates as soon as possible.

[isreallee82]

I think this should be it @fengtality @nikspz

[fengtality]

Hey @isreallee82 apologies, I reviewed this PR and realized that we actually need a different approach to allowances:

Unfortunately, I got a bit confused and asked you add allowances into gateway balance, which is wrong. Since allowances are at the DEX level (spender is specified) while balances are at the wallet/network level, a single command doesn't really work for both. You either have redundant information or missing info.

It would be better to have a separate gateway allowances command that lists the allowances for each connected DEX for the designated wallet, similar to what gateway connect. I think this is worthy of a P2 bounty to implement this functionality correctly and add tests. I'll write up in a separate bounty issue.

[isreallee82]

I agree got abit confused myself @fengtality more reason I included each connectors initially for spender since a connector is required for spender. its alright, should I remove all allowance related, since this pr was initially for feature/ single gateway balance display

[fengtality]

Yes, it's best to remove all the allowances-related code from this PR. I think everything else should still be good to merge in.

On Tue, Jan 9, 2024 at 5:39 PM Ajayi Israel @.***> wrote:

I agree got abit confused myself @fengtality https://github.com/fengtality more reason I included each connectors initially for spender since a connector is required for spender. its alright, should I remove all allowance related, since this pr was initially for feature/ single gateway balance display

— Reply to this email directly, view it on GitHub https://github.com/hummingbot/hummingbot/pull/6743#issuecomment-1884055699, or unsubscribe https://github.com/notifications/unsubscribe-auth/AANWHVUWZXCRVIKGSZKT6FDYNXWMNAVCNFSM6AAAAABBFG7SFGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQOBUGA2TKNRZHE . You are receiving this because you were mentioned.Message ID: @.***>

-- Michael Feng @.*** +1 908 938 8733

[isreallee82]

alright will do that

[isreallee82]

I think the pr is good for a review now

[fengtality]

@isreallee82 @nikspz created bounty here for allowance command https://github.com/hummingbot/hummingbot/issues/6769

[fengtality]

Hey @isreallee82 - I realized that we should pause and rethink these changes to gateway balance and wanted to let you know before you do more work on this PR and the new allowances bounty. The reason is that I didn't consider the case where users add different wallets for different connectors on the same network.

In this case, gateway balance would only show balances for the first connected wallet, and there's no way for users to see balances for the second wallet.

I'll think about this a bit more and clean this up in the allowances bounty issue.

[isreallee82]

True @fengtality Thank you for bringing this to my attention. I agree that pausing to rethink these changes is a prudent step.

[nikspz]

Closing this PR in prior to this one https://github.com/hummingbot/hummingbot/pull/6787