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(prospective-parachains): handle GetMinimumRelayParents signal #4363

Open
wants to merge 17 commits into
base: eclesio/fragment-chain-impl
Choose a base branch
from

Conversation

DanielDDHM
Copy link

@DanielDDHM DanielDDHM commented Dec 3, 2024

Changes

implements AnswerMinimumRelayParentsRequest

Tests

Unit Tests: Core logic is tested in isolation using testify for assertions

go test -tags integration github.com/ChainSafe/gossamer

Issues

Copy link
Member

@EclesioMeloJunior EclesioMeloJunior left a comment

Choose a reason for hiding this comment

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

LGTM, just a small change in the channel sending line, you can send directly w/o the select to ensure that the other part received the message.

@EclesioMeloJunior
Copy link
Member

should wait the merge of branch of #4337

@EclesioMeloJunior EclesioMeloJunior changed the title feat(prospective-parachains): handle GetMinimumRelayParents signal feat(prospective-parachains): handle GetMinimumRelayParents signal Dec 3, 2024
@EclesioMeloJunior EclesioMeloJunior self-requested a review December 3, 2024 20:34
@DanielDDHM DanielDDHM requested a review from haikoschol December 4, 2024 11:31
Copy link
Contributor

@haikoschol haikoschol left a comment

Choose a reason for hiding this comment

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

almost there, just need to pass the right context in TestProspectiveParachains_HandleMinimumRelayParents() :)

@DanielDDHM
Copy link
Author

Currently waiting for #4337

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