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

Rm auto register 2 #89

Merged
merged 7 commits into from
Dec 2, 2024
Merged

Rm auto register 2 #89

merged 7 commits into from
Dec 2, 2024

Conversation

maurolacy
Copy link
Contributor

A small follow-up to #87.

cargo gen-proto is failing btw. @SebastianElvis can you take a look?

@SebastianElvis
Copy link
Member

SebastianElvis commented Nov 28, 2024

From what I see the submodule somehow points to the main branch but not base/consumer-chain-support branch upon gen-proto. The main branch does not contain zone concierge module and that's why gen-proto failed

@SebastianElvis
Copy link
Member

Created a small commit to fix this.

Btw, how about we use git clone to get the babylon repo, and then generate rust proto files? The submodule seems to create a lot of troubles

@maurolacy
Copy link
Contributor Author

maurolacy commented Nov 29, 2024

Hmm, I like the submodule approach better.

It requires some administration though. Here are the steps that I know and use:

  1. Add the submodule (git submodule add).
  2. Select the specific branch / tag you want the submodule to point to, using git checkout inside the submodule's checked out directory.
  3. Commit that change to the module's directory in the main repo.
  4. You can now use git update to "checkout" that committed branch / tag / commit id in the submodule whenever you need to.
  5. git update --remote updates the submodule ref to the latest remote commit in the specified branch. Not very useful, as this is usually not what you want, but still.

@maurolacy
Copy link
Contributor Author

maurolacy commented Nov 29, 2024

I've now committed a change based in the sequence above.

The trick here is always committing the submodule's branch / commit changes into the main repo before running cargo gen-proto.
This is because we have git submodule update as part of the script, btw. We may decide to remove that, which will allow us to generate the proto files from a dirty / uncommitted submodule's state.

@maurolacy
Copy link
Contributor Author

maurolacy commented Nov 29, 2024

Now, after changing / updating the proto files, the apis package is broken. Something related to delegator_unbonding_info vs. delegator_unbonding_sig:

error[E0609]: no field `delegator_unbonding_info` on type `babylon_proto::babylon::btcstaking::v1::BtcUndelegationInfo`
   --> packages/apis/src/btc_staking_api.rs:246:14
    |
246 |             .delegator_unbonding_info
    |              ^^^^^^^^^^^^^^^^^^^^^^^^ unknown field
    |
help: a field with a similar name exists
    |
246 |             .delegator_unbonding_sig
    |              ~~~~~~~~~~~~~~~~~~~~~~~

Can you take a look? 🙏🏼

@maurolacy
Copy link
Contributor Author

maurolacy commented Nov 29, 2024

Hmm, I think I know what's happening. We must point the babylon submodule in this PR to your https://github.com/babylonlabs-io/babylon/tree/feat/permissioned-integration branch.

Will do the change and confirm.

@maurolacy
Copy link
Contributor Author

maurolacy commented Nov 29, 2024

Yes, it works now. After merging the other PR, we can change the submodule ref to the base/consumer-chain-support branch back.

@SebastianElvis
Copy link
Member

Hmm thanks for finding this out. We can change to use base branch after mering the permissioned integrtaion PR

Copy link
Member

@SebastianElvis SebastianElvis left a comment

Choose a reason for hiding this comment

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

Lgtm!

@maurolacy maurolacy merged commit 5f5ba8d into main Dec 2, 2024
3 checks passed
@maurolacy maurolacy deleted the rm-auto-register-2 branch December 2, 2024 07:05
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.

2 participants