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

fix: Switch staking contract not working properly #756

Open
wants to merge 9 commits into
base: staging
Choose a base branch
from

Conversation

mohandast52
Copy link
Collaborator

@mohandast52 mohandast52 commented Feb 11, 2025

Proposed changes

Iason mentioned that it’s not switching as expected, but based on the pattern, it seems to be switching to the default staking program. Interestingly, this issue appears to occur only on Windows. The root cause is still unknown, but I’ve made some updates:

  • Now, the “Active” staking program ID is updated only after the migration happens - here
  • Added a message and a log to check if the “stakingProgramId” is set correctly instead of defaulting - here
  • Refactored some code with comments and improved types to enhance clarity and safety in the future.

Please review and let me know your thoughts on potential root causes.

Types of changes

What types of changes does your code introduce?
Put an x in the boxes that apply

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

@mohandast52 mohandast52 added the bug Something isn't working label Feb 11, 2025
@mohandast52 mohandast52 self-assigned this Feb 11, 2025
@@ -78,8 +80,12 @@ export const useStakingProgram = () => {
selectedStakingProgramMeta,

// all staking programs
allStakingProgramIds: Object.keys(allStakingProgramNameAddressPair),
allStakingProgramAddress: Object.values(allStakingProgramNameAddressPair),
allStakingProgramIds: Object.keys(
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I’ll update the types without using “as” whenever I get the time 😞
(Hate using "as" 🥲)

Comment on lines 188 to 192
setIsMigrating(false);
setActiveStakingProgramId(stakingProgramIdToMigrateTo);
overrideSelectedServiceStatus(MiddlewareDeploymentStatus.DEPLOYING);
goto(Pages.Main);

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

let me know your thoughts on this or 👍 if fine to keep this.

@mohandast52 mohandast52 marked this pull request as ready for review February 11, 2025 12:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants