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.

Sign up for GitHub

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

Jump to bottom

fix: using runner_features and stash_field_from_event inside tests #129

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix: using runner_features and stash_field_from_event inside tests #129

wants to merge 1 commit into from

Conversation

Psycho-Pirate
Copy link
Contributor

Tests are updated to use runner_features and stash_field_from_event, this allows the tests to support multiple lightning implementations at the same time.

@Psycho-Pirate Psycho-Pirate changed the title using runner_features and stash_field_from_event inside tests fix: using runner_features and stash_field_from_event inside tests Dec 7, 2024
@vincenzopalazzo

This comment was marked as off-topic.

Sign in to view
@vincenzopalazzo
Copy link
Collaborator

Thanks @Psycho-Pirate for this but I need to ask why we need this change, and if you can explain it inside the commit body.

IIRC this change is needed when different implementations require different features bit by default. I am right about this?

If yes can you add a detailed description inside it? Can everyone who reads your code use the commit as separate documentation?

@Psycho-Pirate
Copy link
Contributor Author

Sorry for delay, before I digg inside the review, can you please fix the Ci?

I think this might be a core-lightning bug because it is passing with LDK

@Psycho-Pirate
using runner_features and stash_field_from_event inside tests
e37306f 
runner_features provides the features required by an implementation to Lnprototest.
stash_field_from_event allows retrieval of information from the message that was
just decoded within the `ExpectMsg` event.

Using these methods will make Lnprototest more generalised and compatible with other lightning implementations.
@Psycho-Pirate
Copy link
Contributor Author

Thanks @Psycho-Pirate for this but I need to ask why we need this change, and if you can explain it inside the commit body.

IIRC this change is needed when different implementations require different features bit by default. I am right about this?

If yes can you add a detailed description inside it? Can everyone who reads your code use the commit as separate documentation?

Added a descriptive commit message

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Psycho-Pirate @vincenzopalazzo