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

1043 minifront cannot claim unbonding tokens #1066

Merged
merged 9 commits into from
May 8, 2024

Conversation

Valentine1898
Copy link
Contributor

@Valentine1898 Valentine1898 commented May 7, 2024

This is because getUndelegateClaimPlannerRequest cannot recover the IdentityKey from the token's value view. The reason for that is that the ValueView is missing extended_metadata which only get populated for bonding tokens.

Now extended_metadata fills in for unbonding tokens as well

close #1043

@Valentine1898 Valentine1898 linked an issue May 7, 2024 that may be closed by this pull request
import { servicesCtx } from '../ctx/prax';
import { Code, ConnectError } from '@connectrpc/connect';

export const getValidatorInfo: Impl['getValidatorInfo'] = async (req, ctx) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added the getValidatorInfo implementation because it would not be optimal to iterate through the list of all validators for each unbonding balance

Copy link
Contributor

Choose a reason for hiding this comment

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

This is great 👍

Comment on lines +94 to +103
const mockBalancesResponse4 = new BalancesResponse({
balanceView: {
valueView: {
case: 'unknownAssetId',
value: {
amount: { hi: 0n, lo: 3n },
},
},
},
});
Copy link
Contributor Author

Choose a reason for hiding this comment

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

added to make sure that having balances with unknownAssetId does not break anything

Comment on lines 61 to 65
const withValidatorInfo = getBalanceView(new BalancesResponse(balancesResponse)).clone();
if (withValidatorInfo.valueView.case !== 'knownAssetId')
throw new Error(`Unexpected ValueView case: ${withValidatorInfo.valueView.case}`);

withValidatorInfo.valueView.value.extendedMetadata = extendedMetadata;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same way as for delegation tokens

const withValidatorInfo = delegation.balanceView.clone();
if (withValidatorInfo.valueView.case !== 'knownAssetId')
throw new Error(`Unexpected ValueView case: ${withValidatorInfo.valueView.case}`);
withValidatorInfo.valueView.value.extendedMetadata = extendedMetadata;
yield new DelegationsByAddressIndexResponse({ valueView: withValidatorInfo });

Copy link
Collaborator

@grod220 grod220 left a comment

Choose a reason for hiding this comment

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

LGTM

Comment on lines 10 to 14
export const stakingImpl: Pick<Impl, 'getValidatorInfo' | 'validatorInfo' | 'validatorPenalty'> = {
getValidatorInfo,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Would kinda be nice if this was an Omit<Impl, ...>, so that it was kinda in our face what we have not implemented yet

value: validatorInfo.toBinary(),
});

const withValidatorInfo = getBalanceView(new BalancesResponse(balancesResponse)).clone();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why do we need to clone()?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed, it's unnecessary

@Valentine1898 Valentine1898 force-pushed the 1043-minifront-cannot-claim-unbonding-tokens branch from 1e7591b to 6fda723 Compare May 8, 2024 15:45
@Valentine1898 Valentine1898 merged commit 336455b into main May 8, 2024
6 checks passed
@Valentine1898 Valentine1898 deleted the 1043-minifront-cannot-claim-unbonding-tokens branch May 8, 2024 15:52
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.

minifront: cannot claim unbonding tokens
3 participants