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

feat: Automatic consumer registration #34

Merged
merged 22 commits into from
Aug 14, 2024

Conversation

gusin13
Copy link
Collaborator

@gusin13 gusin13 commented Aug 7, 2024

Fixes https://github.com/babylonlabs-io/babylon-private/issues/3

  1. Modifies InstantiateMsg to also receive consumer related info
  2. Stores this metadata in Config store
  3. On ibc_channel_connect loads the config and prepares and send the packet

@gusin13 gusin13 marked this pull request as ready for review August 12, 2024 23:11
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.

Looks good 👍

Copy link
Collaborator

@maurolacy maurolacy left a comment

Choose a reason for hiding this comment

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

LGTM.

contracts/babylon/src/contract.rs Outdated Show resolved Hide resolved
contracts/babylon/src/ibc.rs Outdated Show resolved Hide resolved
contracts/babylon/src/ibc.rs Outdated Show resolved Hide resolved
Comment on lines 52 to 60
// Check if consumer name is empty
if self.consumer_name.trim().is_empty() {
return Err(StdError::generic_err("Consumer name cannot be empty"));
}

// Check if consumer description is empty
if self.consumer_description.trim().is_empty() {
return Err(StdError::generic_err("Consumer description cannot be empty"));
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: Use a contract error proper (MissingConsumerInfo(String) or so).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this fn expects error of type StdError, if we make a custom type then we need to change the fn definition as well?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, you can change the fn signature to return ContractError, and adapt other errors if needed.

@gusin13 gusin13 merged commit 7e941a4 into main Aug 14, 2024
1 check passed
@gusin13 gusin13 deleted the gusin13/consumer-auto-registeration branch August 14, 2024 20:04
Comment on lines +44 to +46
// Update config with consumer information
cfg.consumer_name = msg.consumer_name;
cfg.consumer_description = msg.consumer_description;
Copy link
Collaborator

@maurolacy maurolacy Aug 26, 2024

Choose a reason for hiding this comment

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

Thinking a bit more about this, another option could be to set an empty description (or remove description entirely), and do a query for the consumer's chain id. And then use that as consumer_name.

That way it's not necessary to pass those fields during instantiation. It will also address the issue of consumer name being None, or set but empty. It will also prevent setting a wrong / clashing consumer.

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