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

more robust repairWalletForIncarnation2 #8966

Merged
merged 4 commits into from
Feb 23, 2024
Merged

Conversation

turadg
Copy link
Member

@turadg turadg commented Feb 22, 2024

Description

In testing with production state we noticed new log messages that hadn't appeared in synthetic tests. And the investigation of making invitations revealed that they were never burned.

This addresses those by:

  1. explicitly logging the rejections in repairWalletForIncarnation2
  2. burning the invitations that are made just to infer amounts
  3. pulling amounts from state when possible

It also includes some type coverage improvements made while navigating the code above.

Security Considerations

none

Scaling Considerations

Prevents some wasted retention of unused invitations

Documentation Considerations

n/a

Testing Considerations

we should see the new log messages in upgrade-14 testing

Upgrade Considerations

To be included in dev-upgrade-14 cc @mhofman

Copy link
Contributor

@Chris-Hibbert Chris-Hibbert left a comment

Choose a reason for hiding this comment

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

Thanks for the clean-ups!

@turadg turadg added the automerge:rebase Automatically rebase updates, then merge label Feb 23, 2024
@turadg turadg mentioned this pull request Feb 23, 2024
@mergify mergify bot merged commit c2a8d06 into master Feb 23, 2024
75 checks passed
@mergify mergify bot deleted the ta/smart-wallet-repair branch February 23, 2024 01:28
Comment on lines +648 to +652
let invitationAmount = state.offerToUsedInvitation.get(
// @ts-expect-error older type allowed number
offerSpec.id,
);
if (invitationAmount) {
Copy link
Member

Choose a reason for hiding this comment

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

This will throw if the key doesn't exist. It needs to be a .has check instead

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge:rebase Automatically rebase updates, then merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants