-
Notifications
You must be signed in to change notification settings - Fork 17
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
Update core dependencies to v0.70.3 #839
Changes from 19 commits
b32c81e
a8127ca
b059118
c3b06cd
d3eb48e
30ed73e
d78eb7c
b97cc09
69a0d12
512a1fb
d2db570
d0bbdf7
d992488
a952c71
b333ac7
cdc8e16
9b48ccb
01eb8e3
d741b34
1f02d83
e32f590
3f07204
2f3f7ca
f596006
87b6436
b9fc0f4
64a1525
cdfc04b
d581b4f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,6 @@ | ||
|
||
PRAX=lkpmkhpnhknhmibgnmmhdhgdilepfghe | ||
IDB_VERSION=30 | ||
IDB_VERSION=31 | ||
USDC_ASSET_ID="reum7wQmk/owgvGMWMZn/6RFPV24zIKq3W6In/WwZgg=" | ||
MINIFRONT_URL=https://app.testnet.penumbra.zone/ | ||
PENUMBRA_NODE_PD_URL=https://grpc.testnet.penumbra.zone/ |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -145,13 +145,18 @@ export class BlockProcessor implements BlockProcessorInterface { | |
const startHeight = fullSyncHeight ? fullSyncHeight + 1n : 0n; | ||
let latestKnownBlockHeight = await this.querier.tendermint.latestBlockHeight(); | ||
|
||
// In the `for` loop below, we only update validator infos once we've | ||
// reached the latest known epoch. This means that, if a user is syncing for | ||
// the first time, they could experience a broken UI until the latest known | ||
// epoch is reached, since they may have delegation tokens but no validator | ||
// info to go with them. So we'll update validator infos at the beginning of | ||
// sync as well, and force the rest of sync to wait until it's done. | ||
if (startHeight === 0n) await this.updateValidatorInfos(0n); | ||
if (startHeight === 0n) { | ||
// In the `for` loop below, we only update validator infos once we've | ||
// reached the latest known epoch. This means that, if a user is syncing | ||
// for the first time, they could experience a broken UI until the latest | ||
// known epoch is reached, since they may have delegation tokens but no | ||
// validator info to go with them. So we'll update validator infos at the | ||
// beginning of sync as well, and force the rest of sync to wait until | ||
// it's done. | ||
await this.updateValidatorInfos(0n); | ||
|
||
await this.indexedDb.addEpoch(0n); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Previously, we were skipping saving the first epoch to the database. This fixes that bug. |
||
} | ||
|
||
// this is an indefinite stream of the (compact) chain from the network | ||
// intended to run continuously | ||
|
@@ -273,7 +278,7 @@ export class BlockProcessor implements BlockProcessorInterface { | |
|
||
const isLastBlockOfEpoch = !!compactBlock.epochRoot; | ||
if (isLastBlockOfEpoch) { | ||
void this.handleEpochTransition(compactBlock.height, latestKnownBlockHeight); | ||
await this.handleEpochTransition(compactBlock.height, latestKnownBlockHeight); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Needed to If that took a long time, for some reason, and in the meantime the block processor had already gotten to the next epoch, we could have two epochs with the same epoch index. This change makes sure that epoch indexes increment in the correct order. |
||
} | ||
} | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -75,10 +75,11 @@ describe('AssetMetadataById request handler', () => { | |
expect(metadataByIdResponse.denomMetadata?.equals(metadataFromNode)).toBeTruthy(); | ||
}); | ||
|
||
test('should fail to get metadata when metadata not found in idb and node', async () => { | ||
test('should return an empty response when metadata not found in idb and node', async () => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There's really no need to throw, since it's acceptable to return an empty response from this RPC method. So I changed the behavior. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Response on other comment |
||
mockIndexedDb.getAssetsMetadata?.mockResolvedValue(undefined); | ||
mockShieldedPool.assetMetadataById.mockResolvedValueOnce(undefined); | ||
await expect(assetMetadataById(request, mockCtx)).rejects.toThrow(); | ||
const response = new AssetMetadataByIdResponse(await assetMetadataById(request, mockCtx)); | ||
expect(response.equals(new AssetMetadataByIdResponse())).toBeTruthy(); | ||
}); | ||
|
||
test('should fail if assetId is missing in request', async () => { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -458,12 +458,14 @@ export class IndexedDb implements IndexedDbInterface { | |
}); | ||
} | ||
|
||
async addEpoch(startHeight: bigint, index?: bigint): Promise<void> { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We're never actually using the |
||
if (index === undefined) { | ||
const cursor = await this.db.transaction('EPOCHS', 'readonly').store.openCursor(null, 'prev'); | ||
const previousEpoch = cursor?.value ? Epoch.fromJson(cursor.value) : undefined; | ||
index = previousEpoch?.index !== undefined ? previousEpoch.index + 1n : 0n; | ||
} | ||
/** | ||
* Adds a new epoch with the given start height. Automatically sets the epoch | ||
* index by finding the previous epoch index, and adding 1n. | ||
*/ | ||
async addEpoch(startHeight: bigint): Promise<void> { | ||
const cursor = await this.db.transaction('EPOCHS', 'readonly').store.openCursor(null, 'prev'); | ||
const previousEpoch = cursor?.value ? Epoch.fromJson(cursor.value) : undefined; | ||
const index = previousEpoch?.index !== undefined ? previousEpoch.index + 1n : 0n; | ||
jessepinho marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
const newEpoch = { | ||
startHeight: startHeight.toString(), | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,10 +13,6 @@ export const UndelegateComponent = ({ value }: { value: Undelegate }) => { | |
label='Undelegate' | ||
visibleContent={ | ||
<ActionDetails> | ||
<ActionDetails.Row label='Epoch index'> | ||
{value.startEpochIndex.toString()} | ||
</ActionDetails.Row> | ||
Comment on lines
-16
to
-18
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a little strange to me: the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can you leave the code here and just style it hidden or comment it out? |
||
|
||
{!!value.delegationAmount && ( | ||
<ActionDetails.Row label='Delegation amount'> | ||
{joinLoHiAmount(value.delegationAmount).toString()} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -33,7 +33,7 @@ export const ValueViewComponent = ({ | |
const value = view.valueView.value; | ||
const metadata = view.valueView.value.metadata; | ||
const amount = value.amount ?? new Amount(); | ||
const exponent = getDisplayDenomExponent(metadata); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Reviewers: I could use some help here. When a user clicks the Undelegate button, the transaction approval dialog opens. We try to fetch the metadata for the undelegation tokens they'll be getting, but that will always come back empty, since the unbonding token type is now specific to the block height at which unbonding starts. That is, the unbonding token name is There will never be token metadata for an unbonding token until after the transaction has been committed to the chain, since the token type is created when the transaction is committed. But that means that the undelegation tokens are shown as "Unknown asset" in the transaction approval dialog: Perhaps we should generate the unbonding token metadata on the fly here? If so, could someone point me in the right direction for how to do that? (That said, I'd love to get this PR merged before handling that, since all delegation is blocked until this PR is merged.) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Another wrinkle to all this: this issue, where the metadata request always returns an empty response for an undelegation token that doesn't exist yet, wasn't happening before when undelegation tokens were tied to epochs rather than block heights. I don't know how that's possible. We should have had the same problem back then, right? That is:
And yet, this wasn't happening before — no error was being thrown. It only started with this change, where unbonding tokens are tied to block height rather than epoch index. I'm investigating this, but it's a bit concerning since I don't understand why it worked before! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The
Umm, no idea! 😅 |
||
const exponent = getDisplayDenomExponent.optional()(metadata); | ||
// The regex trims trailing zeros which toFormat adds in | ||
const formattedAmount = fromBaseUnitAmount(amount, exponent) | ||
.toFormat(6) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Needed for the update to unbonding tokens, which now have a different symbol