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

Bundle mainnet genesis state #165

Merged
merged 6 commits into from
Aug 20, 2024
Merged

Bundle mainnet genesis state #165

merged 6 commits into from
Aug 20, 2024

Conversation

grod220
Copy link
Contributor

@grod220 grod220 commented Aug 19, 2024

References #161, pairs with penumbra-zone/web#1706

Copy link

changeset-bot bot commented Aug 19, 2024

⚠️ No Changeset found

Latest commit: dfa99dc

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

packages/context/src/index.ts Outdated Show resolved Hide resolved
packages/context/src/genesis.test.ts Outdated Show resolved Hide resolved
@TalDerei
Copy link
Contributor

closing #163 in favor of this.

@grod220
Copy link
Contributor Author

grod220 commented Aug 19, 2024

@TalDerei feel free to take this over as it pairs with your other work

@hdevalence
Copy link
Contributor

Looking at the rust code here https://github.com/penumbra-zone/web/blob/main/packages/wasm/crate/src/view_server.rs#L116 , isn't the compact block passed into wasm as a binary-proto-encoded byte array? in that case, wouldn't it be best to store the compact block in that format to avoid serialization overhead, and which would also be smaller to include in the extension manifest?

@TalDerei
Copy link
Contributor

@TalDerei feel free to take this over as it pairs with your other work

thanks taking over PR 👍

@TalDerei
Copy link
Contributor

TalDerei commented Aug 20, 2024

Looking at the rust code here https://github.com/penumbra-zone/web/blob/main/packages/wasm/crate/src/view_server.rs#L116 , isn't the compact block passed into wasm as a binary-proto-encoded byte array? in that case, wouldn't it be best to store the compact block in that format to avoid serialization overhead, and which would also be smaller to include in the extension manifest?

In theory, storing the compact block object as a binary-proto-encoded byte array should be more efficient in terms of both serialization and space costs.

In terms of serialization, we'll still need to perform a deserialization operation in the extension when fetching the genesis file, CompactBlock.fromBinary(new Uint8Array(binaryData)), since the block processor processes and perform checks on the in-memory compact block object before converting it to the byte array that wasm consumes. However, deserializing from binary should be more efficient than deserializing from JSON.

In terms of space, the compact block object for the genesis block is ~10 MB, while its JSON representation is ~18 MB. Surprisingly, when serialized to a binary-proto-encoded byte array using compactBlock.toBinary() and saved to a binary file, it takes up ~150 MB.

Screenshot 2024-08-19 at 6 23 12 PM

cc @turbocrime

@hdevalence
Copy link
Contributor

I'm not sure where those numbers are coming from. The genesis compact block is 8035648 bytes. See penumbra-zone/penumbra#4824 for code to generate the data

@TalDerei
Copy link
Contributor

I'm not sure where those numbers are coming from. The genesis compact block is 8035648 bytes. See penumbra-zone/penumbra#4824 for code to generate the data

strange blowup, I was simply logging the binary representation of the compact block and manually saving to a file.

@grod220
Copy link
Contributor Author

grod220 commented Aug 20, 2024

Tested this out locally and things are working for me. @TalDerei after publishing the latest packages in penumbra-zone and pulling them in here, think you can merge.

In general, it's nice this saves the 8mb download. However, for a full sync the user will still have to download another +60mb. Connectrpc downloads all of those in parallel to the block sync logic. That said, it's not really noticed on my machine as the CPU/IO tasks are the bottleneck and those downloaded blocks are just waiting to be processed.

@TalDerei
Copy link
Contributor

TalDerei commented Aug 20, 2024

That said, it's not really noticed on my machine as the CPU/IO tasks are the bottleneck and those downloaded blocks are just waiting to be processed.

echoing the same observation on M3 pro

@@ -103,7 +104,15 @@ export class Services implements ServicesInterface {
idbConstants: indexedDb.constants(),
});

// Dynamically fetch binary file for genesis compact block
Copy link
Contributor

@TalDerei TalDerei Aug 20, 2024

Choose a reason for hiding this comment

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

fetch api goes through the browser's network stack for local files, and there seems to be overhead in setting up the request, but latency is negligible

Screenshot 2024-08-20 at 8 18 39 AM

@TalDerei TalDerei merged commit edd6fac into main Aug 20, 2024
3 checks passed
@TalDerei TalDerei deleted the bundle-genesis-block branch August 20, 2024 19:18
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.

4 participants