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

feature: Protos #237

Closed
wants to merge 3 commits into from
Closed

feature: Protos #237

wants to merge 3 commits into from

Conversation

gbarkhatov
Copy link
Contributor

  • Adds compiled protobuf files to src/proto
  • If we want to update them - just download the latest git repo, put babylon at the top level and run npm run build:proto

managed:
enabled: true
plugins:
- plugin: buf.build/community/stephenh-ts-proto
Copy link
Contributor

Choose a reason for hiding this comment

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

Any reason to use this particular plugin? It generates a lot of duplicated data which will blow up our bundle size. From the another hand default plugin only generates schemas + types which can be used together with fromBinary method

Copy link
Collaborator

Choose a reason for hiding this comment

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

Agree with David on this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@totraev @jrwbabylonlab
We can configure our protobuf files generation in multiple ways. The reason I chose this approach is

  • it's the easiest, generates everything
  • it was already established and working on btc-staking-dashboard application
    It leads to https://i.imgur.com/lK7Xr86.png, generates all the types for the dependencies, etc.

We can play around with config on https://github.com/babylonlabs-io/babylon-proto-ts

@totraev I am not sure whether or not usage of .fromBinary will be handy or that the current approach bloats the bundle size. But you can create an issue on that repo describing the pros of the suggested approach in comparison with the current that is already set up

cp -r "$BABYLON_PROTO" "$PROTO_DIR"
echo "Compiling Babylon proto files to TS files"
cd "$SCRIPTS_DIR" && npx buf generate proto
cd ../..
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: i think this can be simplified to npx buf generate "$PROTO_DIR" ?


# Clean up
rm -r "$PROTO_DIR"
rm -rf "$BABYLON_REPO_DIR"
Copy link
Collaborator

Choose a reason for hiding this comment

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

would be nice t perform this clean up even if any of the above command were failed/existed.
try with trap?

trap 'rm -rf "$PROTO_DIR" "$BABYLON_REPO_DIR"' EXIT

@jrwbabylonlab jrwbabylonlab removed the request for review from jeremy-babylonlabs October 21, 2024 04:09
@gbarkhatov
Copy link
Contributor Author

Minor update - this PR might be not needed as we move to the npm library and a separate repository

@gbarkhatov
Copy link
Contributor Author

Closing this in favor of babylon-proto-ts

@gbarkhatov gbarkhatov closed this Oct 21, 2024
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.

3 participants