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

remove repairWalletForIncarnation2 from smartWallet #9404

Closed
Chris-Hibbert opened this issue May 23, 2024 · 0 comments · Fixed by #10319
Closed

remove repairWalletForIncarnation2 from smartWallet #9404

Chris-Hibbert opened this issue May 23, 2024 · 0 comments · Fixed by #10319
Assignees
Labels
enhancement New feature or request hygiene Tidying up around the house wallet

Comments

@Chris-Hibbert
Copy link
Contributor

What is the Problem Being Solved?

#8445 refactored smartWallets and added code to backfill existing wallets. That repair code ran in Upgrade 15, and is now obsolete.

Description of the Design

Remove it. It would take two steps to also remove the flag upgradeToIncarnation2Key. I think we can leave the flag in place. The space costs are negligible.

Security Considerations

None

Scaling Considerations

None

Test Plan

No behavior changes

Upgrade Considerations

No urgency on getting this on-chain, but it would be nice to clean up the repo.

@Chris-Hibbert Chris-Hibbert added enhancement New feature or request wallet hygiene Tidying up around the house labels May 23, 2024
@Chris-Hibbert Chris-Hibbert self-assigned this May 25, 2024
@mergify mergify bot closed this as completed in #10319 Oct 23, 2024
mergify bot added a commit that referenced this issue Oct 23, 2024
closes: #9404

## Description

In #8445, we repaired legacy smartWallets. This removes the code that ran in upgrade15 that did the cleanup.

### Security Considerations

None

### Scaling Considerations

None

### Documentation Considerations

None

### Testing Considerations

No tests should be removed.

### Upgrade Considerations

There is no hurry to merge this code on chain. If the PR merges soon, it'll be included in upgrade 18.
mergify bot pushed a commit that referenced this issue Oct 29, 2024
refs: #9404 
refs: #10336
refs: #10319



## Description

#10319 cleaned up some dead code in smartWallets, and expected the changes to be included in Upgrade 18, because the smartWallet proposal was included in `upgrade.go`. #10336 cleaned up proposals that were left over from Upgrade 17. This reinstates the upgrade for wallets.

### Security Considerations

The cleaned up code is benign. it was designed to run once and has served its purpose. Removing it shouldn't affect anything.

### Scaling Considerations

No scaling consequences.

### Documentation Considerations

None

### Testing Considerations

The wallet upgrade was included in A3P-integration testing until three days ago. #10319 has been tested since 5 days ago without incident. removing benign dead code doesn't require extra tests.

### Upgrade Considerations

There is no *requirement* that this change be included in U18, so if it causes problems or gives anyone the willies, it can wait for the next release, but it's ready now.
mujahidkay pushed a commit that referenced this issue Oct 30, 2024
refs: #9404
refs: #10336
refs: #10319

The cleaned up code is benign. it was designed to run once and has served its purpose. Removing it shouldn't affect anything.

No scaling consequences.

None

The wallet upgrade was included in A3P-integration testing until three days ago. #10319 has been tested since 5 days ago without incident. removing benign dead code doesn't require extra tests.

There is no *requirement* that this change be included in U18, so if it causes problems or gives anyone the willies, it can wait for the next release, but it's ready now.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request hygiene Tidying up around the house wallet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant