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: TAO staking #1682

Open
wants to merge 64 commits into
base: dev
Choose a base branch
from
Open

feat: TAO staking #1682

wants to merge 64 commits into from

Conversation

UrbanWill
Copy link

@UrbanWill UrbanWill commented Nov 3, 2024

Description

Implements TAO staking

⚠️ Warning

Techical

  • Renamed folder/components from Nomination pool bond specific to generic bonding names, eg: NomPoolBondModal -> BondModal
  • Created bittensor and nompool folders inside a new hooks folder for organization purposes
  • Created useGetStakeInfo and useGetUnbondInfo hooks to support the wizard hooks. These hooks are responsible for returning all the information that is specific to an Asset that is being staked/unbonded.
  • Removed fake fee estimate, use a fee estimation that defaults to minJoinBond if the input amount is 0 or less than min for that pool, disable form submission if planks is < 0n

TODO's

  • Hooks for fetching TAO delegator data
  • Pop up UI for selecting delegator
  • Dashboard UI for selecting a delegator
  • Refactor dashboard asset view to display the amount stake per Delegator
  • Hook to track the block number when the user unstaked the total balance from a delegator (used for 360 blocks cooldown)

🖼️ Screenshots:

Screen.Recording.2024-11-14.at.20.33.17.mov

fetch payload with 0n plancks and invalidate form if pancks is < 0n
alecdwm
alecdwm previously approved these changes Nov 19, 2024
Copy link
Member

@alecdwm alecdwm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👌

@chidg
Copy link
Contributor

chidg commented Nov 19, 2024

Looking great, a few small things to add re: error handling of fetch requests.

When no TAOSTATS_API_KEY is configured, or else the fetchBittensorInfiniteValidators function fails for some reason, it would be good to display a nice message like "Unable to fetch validators" rather than just an empty screen

Screenshot 2024-11-19 at 6 24 05 PM

Copy link
Contributor

@0xKheops 0xKheops left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

some minor things, please check below.

another that didnt fit to a particular part of the code is that i see bittensor staking related hooks being called a lot for no reason, especially in AssetDetails
calling any of them loads bittensor sapi object which downloads the full metadata.
Unless a bittensor balance is detected, it would be nice to make sure those hooks do nothing.
Note: at this time even if if bittensor is disabled in settings, they will fetch metadata and execute all the logic

Copy link
Contributor

@chidg chidg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Few more comments, apologies - these got lost in vs code PR review process somewhere

Copy link
Contributor

@0xKheops 0xKheops left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, one more change needed in TxProgress :)

networkIdOrHash,
onClose,
className,
onSuccess,
Copy link
Contributor

@0xKheops 0xKheops Nov 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry i didnt dig this thing enough last time.
It feels like success detection shouldnt originate from here, which is a UI component that is meant to support both EVM and substrate.

Option 1 : use useTransaction(hash) from useUnbondWizard() to detect success from there, without needing any callbacks, and rollback all changes to this component
Option 2 : move the success check from TxProgressSubstrate up to TxProgress, so it can support both substrate and evm

Preference for option 1 though

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.

4 participants