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: migrate business logic in dApp staking v2 to v2(new) architecture #662

Merged
merged 14 commits into from
Feb 8, 2023

Conversation

impelcrypto
Copy link
Member

@impelcrypto impelcrypto commented Jan 11, 2023

Pull Request Summary

feat: migrate business logic in dApp staking v2 to v2(new) architecture

Check list

  • contains breaking changes
  • adds new feature
  • modifies existing feature (bug fix or improvements)
  • relies on other tasks
  • documentation changes
  • tested on mobile devices

Release notes:

  • feat: migrate business logic in dApp staking v2 to v2(new) architecture

This pull request makes the following changes:

Changes

  • (ex: Change document B)

@github-actions
Copy link

github-actions bot commented Jan 11, 2023

Visit the preview URL for this PR (updated for commit 80b1a9d):

https://astar-apps--pr662-feat-dapps-staking-v-5cnpdvye.web.app

(expires Tue, 14 Feb 2023 09:12:00 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: dd76fe72958fe2910fef9d53f0b4539b82b849db

for (let i = 0; i < batchTxsRef.length; i++) {
const tx = batchTxsRef[i];
console.log('tx.weight', tx.weight.toString());
Copy link
Member Author

@impelcrypto impelcrypto Jan 11, 2023

Choose a reason for hiding this comment

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

Hi @bobo-k2
I found there is a breaking change in tx.weight because WeightV2(?) is coming. Could you help take a look at update the logic? We can't claim rewards on Shibuya now.

=Shibuya (WeightV2)=

=Astar/Shiden=

Copy link
Contributor

Choose a reason for hiding this comment

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

Sure, I will take a look

Copy link
Contributor

Choose a reason for hiding this comment

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

Fixed in #671

@impelcrypto impelcrypto changed the title feat: migrate business logic in dapp staking v2 to v2 architecture feat: migrate business logic in dapp staking v2 to v2(new) architecture Jan 11, 2023
@impelcrypto impelcrypto changed the title feat: migrate business logic in dapp staking v2 to v2(new) architecture feat: migrate business logic in dApp staking v2 to v2(new) architecture Jan 11, 2023
@sirius651 sirius651 mentioned this pull request Jan 12, 2023
6 tasks
sirius651 and others added 6 commits January 13, 2023 16:54
* add getStakeInfo function in repo

* migrate getStake function to v2

* fix: modified fetching 'My staking information' logic when users change the account (#666)

Co-authored-by: impelcrypto <92044428+impelcrypto@users.noreply.github.com>
* diminish snowpack & put space on tvl text (#663)

* feat:Support for BNC and vDOT assets (#658)

* feat:Support for BNC and vDOT assets

* Fix some errors

* Update BifrostXcmRepository.ts

* Add some constants

* 0.0.9

* feat: updated japanese translation (#665)

* go back logo (#667)

* migrate for useUnbonding hook

Co-authored-by: NingBo Wang <2536935847@qq.com>
Co-authored-by: github-actions <github-actions@users.noreply.github.com>
Co-authored-by: impelcrypto <92044428+impelcrypto@users.noreply.github.com>
@sirius651
Copy link
Contributor

@impelcrypto @bobo-k2 Finished migrate for useUnbonding by following to useClaim. please review. and I just wondering isn't it the right direction to hide all calls related to api? It is used in both hooks and v2s, but what are the advantages of this?

@bobo-k2
Copy link
Contributor

bobo-k2 commented Jan 16, 2023

@impelcrypto @bobo-k2 Finished migrate for useUnbonding by following to useClaim. please review. and I just wondering isn't it the right direction to hide all calls related to api? It is used in both hooks and v2s, but what are the advantages of this?

Main idea was to separate concerns so we can write unit tests and make code maintenance easier.. Eg. on Sub0 there were talks about Polkadot.js replacement. If sometimes in the future we decide to replace Polkadot.js, because of the better api we will need to change repositories only. The rest of the code should be fine.
I am open for discussion about UI architecture direction. We can have a call sometimes this week if needed.

@impelcrypto impelcrypto marked this pull request as ready for review January 16, 2023 08:07
@impelcrypto
Copy link
Member Author

impelcrypto commented Jan 16, 2023

My Available to claim hasn't updated (without refresh the page) after the claiming transaction has been done. Let's take a look 🤔

image

@sirius651
Copy link
Contributor

My Available to claim hasn't updated (without refresh the page) after the claiming transaction has been done. Let's take a look 🤔

when I tried it on my end, it works well. @bobo-k2 can you reproduce this?

@andabak andabak requested a review from bobo-k2 January 31, 2023 12:12
@bobo-k2
Copy link
Contributor

bobo-k2 commented Feb 6, 2023

My Available to claim hasn't updated (without refresh the page) after the claiming transaction has been done. Let's take a look 🤔

when I tried it on my end, it works well. @bobo-k2 can you reproduce this?

It works on my side. Available to claim refreshes after Claim operation is performed

@bobo-k2
Copy link
Contributor

bobo-k2 commented Feb 6, 2023

It looks like unbonding is not working for some dApps. Try the following on Shibuya:

  • click unbond (I tested with ADAO dApp)
  • enter amount and sign transaction
  • spinner will show
  • spinner will hide, but unbonding dialog stays opened and no unbonding performed.

image

I tried some other dApps and unbond without problem. Also noticed that Unbonding tab didn't show after successfull unbodning. I needed to refresh the the portal. I can't reproduce this anymore, but it is worth checking

@impelcrypto
Copy link
Member Author

My Available to claim hasn't updated (without refresh the page) after the claiming transaction has been done. Let's take a look 🤔

@sirius651 @bobo-k2 It works properly now

@impelcrypto
Copy link
Member Author

It looks like unbonding is not working for some dApps. Try the following on Shibuya:

@bobo-k2 @sirius651 I've fixed this issue. Please test it again.

Copy link
Contributor

@sirius651 sirius651 left a comment

Choose a reason for hiding this comment

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

it works well for me

Copy link
Contributor

@bobo-k2 bobo-k2 left a comment

Choose a reason for hiding this comment

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

LGTM

@impelcrypto
Copy link
Member Author

@sirius651 You can merge this PR anytime :D

@sirius651 sirius651 merged commit f087772 into main Feb 8, 2023
@sirius651 sirius651 deleted the feat/dapps-staking-v2 branch February 8, 2023 02:11
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.

3 participants