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.

Sign up for GitHub

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

Jump to bottom

feat: Add StakeSdkProvider with stake-sdk #11668

Merged
merged 16 commits into from
Oct 18, 2024
Merged

feat: Add StakeSdkProvider with stake-sdk #11668

merged 16 commits into from
Oct 18, 2024
+2,427 −21

Conversation

siibars
Copy link
Contributor

@siibars siibars commented Oct 7, 2024 •
edited
Loading

Description

This pr introduces the StakeSdkProvider which eposes the stake-sdk to be used and shared by different stake components when included in a StakeProvider.
The stake-sdk exposes on & off chain service based for different experiences (pooled staking, validator staking..)

Related issues

Fixes:
STAKE-799

Manual testing steps

  1. clone the repo
  2. yarn install
  3. run npx jest app/components/UI/Stake/sdk/UseSdkProvider.test.tsx

Screenshots/Recordings

Before

After

Pre-merge author checklist

Pre-merge reviewer checklist

  • I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed).
  • I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Oct 7, 2024 •
edited
Loading

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

@socket-security Socket Security
Copy link

socket-security bot commented Oct 7, 2024 •
edited
Loading

New dependencies detected. Learn more about Socket for GitHub ↗︎

Package New capabilities Transitives Size Publisher
npm/@metamask/stake-sdk@0.2.11 None 0 165 kB metamaskbot

View full report↗︎

Matt561
Matt561 reviewed Oct 8, 2024
View reviewed changes
app/components/UI/Stake/Views/InputView/StakeInputView.tsx Outdated Show resolved Hide resolved
app/components/UI/Stake/Views/InputView/StakeInputView.tsx Outdated Show resolved Hide resolved
app/components/UI/Stake/Views/InputView/StakeInputView.tsx Outdated Show resolved Hide resolved
app/components/UI/Stake/Views/InputView/StakeInputView.tsx Outdated Show resolved Hide resolved
app/components/UI/Stake/Views/InputView/StakeInputView.tsx Outdated Show resolved Hide resolved
app/components/UI/Stake/sdk/index.tsx Outdated Show resolved Hide resolved
app/components/UI/Stake/sdk/index.tsx Outdated Show resolved Hide resolved
app/reducers/stake/index.ts Outdated Show resolved Hide resolved
amitabh94
amitabh94 requested changes Oct 9, 2024
View reviewed changes
Copy link
Contributor

@amitabh94 amitabh94 left a comment

Choose a reason for hiding this comment

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

This PR needs some more work in terms of accessing SDK methods and state management.
Since we already have the package imported, it should be fairly simple to just call those methods by instantiating the sdk without adding a lot of context to it .

I would recommend breaking the analytics part out of this PR and adding tests for those in a separate PR which we can use.

@jclancy93 It might be good for you to have a look at this PR as well and see if I might be missing anything or if there is a gap in my understanding.

app/actions/stake/index.ts Outdated Show resolved Hide resolved
app/actions/stake/index.ts Outdated Show resolved Hide resolved
app/components/UI/Stake/Views/InputView/StakeInputView.tsx Outdated Show resolved Hide resolved
app/components/UI/Stake/sdk/index.tsx Outdated Show resolved Hide resolved
app/components/UI/Stake/sdk/index.tsx Outdated Show resolved Hide resolved
app/components/UI/Tokens/index.test.tsx Outdated Show resolved Hide resolved
app/core/redux/slices/stake/index.ts Outdated Show resolved Hide resolved
@siibars siibars marked this pull request as ready for review October 11, 2024 14:55
@siibars siibars requested review from a team as code owners October 11, 2024 14:55
amitabh94
amitabh94 requested changes Oct 15, 2024
View reviewed changes
Copy link
Contributor

@amitabh94 amitabh94 left a comment

Choose a reason for hiding this comment

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

Requesting some changes due to changes in sdk package and some other minor comments.

app/components/UI/Stake/sdk/index.tsx Outdated Show resolved Hide resolved
app/components/UI/Stake/sdk/index.tsx Outdated Show resolved Hide resolved
app/components/UI/Stake/sdk/index.tsx Outdated Show resolved Hide resolved
app/core/Analytics/MetaMetrics.events.ts Outdated Show resolved Hide resolved
app/components/UI/Stake/sdk/index.tsx Outdated Show resolved Hide resolved
@siibars siibars added the Run Smoke E2E Triggers smoke e2e on Bitrise label Oct 17, 2024
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Oct 17, 2024 •
edited by metamaskbot
Loading

https://bitrise.io/ Bitrise

✅✅✅ pr_smoke_e2e_pipeline passed on Bitrise! ✅✅✅

Commit hash: 6db8da1
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/dd648b52-1f58-4931-9862-97507e50a5c6

Note

  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

@siibars siibars changed the title Feat/stake 799 feat: stake 799 Oct 17, 2024
@siibars siibars changed the title feat: stake 799 feat: Add StakeSdkProvider with stake-sdk Oct 17, 2024
@siibars siibars added the No QA Needed Apply this label when your PR does not need any QA effort. label Oct 17, 2024
@siibars
Copy link
Contributor Author

siibars commented Oct 17, 2024

I have read the CLA Document and I hereby sign the CLA

@siibars siibars added Run Smoke E2E Triggers smoke e2e on Bitrise and removed Run Smoke E2E Triggers smoke e2e on Bitrise labels Oct 17, 2024
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Oct 17, 2024 •
edited by metamaskbot
Loading

https://bitrise.io/ Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: d85c30d
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/2b842b17-6db3-465f-a8cf-94732131fa35

Note

  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

Tip

  • Check the documentation if you have any doubts on how to understand the failure on bitrise

@siibars siibars dismissed stale reviews from Matt561 and amitabh94 via 65c72c8 October 17, 2024 16:01
@cortisiko cortisiko added Run Smoke E2E Triggers smoke e2e on Bitrise and removed Run Smoke E2E Triggers smoke e2e on Bitrise labels Oct 17, 2024
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Oct 17, 2024 •
edited by metamaskbot
Loading

https://bitrise.io/ Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: e9d388d
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/667db2da-01cf-43b9-8b8d-6d75cac20a59

Note

  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

Tip

  • Check the documentation if you have any doubts on how to understand the failure on bitrise

@siibars siibars added Run Smoke E2E Triggers smoke e2e on Bitrise and removed Run Smoke E2E Triggers smoke e2e on Bitrise labels Oct 17, 2024
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Oct 17, 2024 •
edited by metamaskbot
Loading

https://bitrise.io/ Bitrise

✅✅✅ pr_smoke_e2e_pipeline passed on Bitrise! ✅✅✅

Commit hash: 0d8c622
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/f2e360b4-7b24-4b00-b987-941cffd36f46

Note

  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

@sonarcloud SonarCloud
Copy link

sonarcloud bot commented Oct 17, 2024

Quality Gate Passed Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
71.4% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

cortisiko
cortisiko approved these changes Oct 17, 2024
View reviewed changes
Copy link
Member

@cortisiko cortisiko left a comment

Choose a reason for hiding this comment

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

✅ ✅

@Matt561 Matt561 self-requested a review October 17, 2024 23:40
@siibars siibars added this pull request to the merge queue Oct 18, 2024
Merged via the queue into main with commit 3ab33a3 Oct 18, 2024
43 checks passed
@siibars siibars deleted the feat/stake-799 branch October 18, 2024 08:07
@github-actions github-actions bot locked and limited conversation to collaborators Oct 18, 2024
@metamaskbot metamaskbot added the release-7.35.0 Issue or pull request that will be included in release 7.35.0 label Oct 18, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@tommasini tommasini tommasini left review comments

@cortisiko cortisiko cortisiko approved these changes

@Matt561 Matt561 Matt561 approved these changes

@amitabh94 amitabh94 amitabh94 left review comments

Assignees
No one assigned
Labels
release-7.35.0 Issue or pull request that will be included in release 7.35.0 Run Smoke E2E Triggers smoke e2e on Bitrise team-stake
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@siibars @amitabh94 @cortisiko @Matt561 @tommasini @metamaskbot