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

Avoid repeated state transition validation #128

Merged
merged 1 commit into from
Jan 23, 2023

Conversation

zoedberg
Copy link
Contributor

This PR fixes #122 by adding a memory index.

The fix may be improved, but we decided to do the minimal possible fix that doesn't break APIs or change the code logic too much.

I don't think much more effort is needed here since there is already #123 which purpose is to make a much bigger improvement and that will happen when breaking changes will be possible.

Copy link
Member

@dr-orlovsky dr-orlovsky left a comment

Choose a reason for hiding this comment

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

ACK 634331d

@dr-orlovsky
Copy link
Member

This is for sure an improvement (and I am going to merge it), but it seems this is not a solution addressing problem from #122

It seems this code avoids repeated validation that state transition is a part of the anchor - but not the validation that the bitcoin transaction commits to the anchor (which what takes an electrum call).

@zoedberg
Copy link
Contributor Author

@dr-orlovsky Are you sure about this? Looking at the code the only call to electrum I see is here, in validate_graph_node, which is being called only here, in validate_branch, inside the if clause where I added the new memory index.

Some calls that check the anchor against the bundle are still repeated but those should not use electrum, so I think we can address those as part of #123.

@dr-orlovsky
Copy link
Member

Oh right, I visually missed the call to validate_graph_node focusing on anchor.convolve one, thinking it was the call optimized. Than you are right!

@dr-orlovsky dr-orlovsky merged commit 5eb142c into RGB-WG:master Jan 23, 2023
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.

Avoid repeated state transition validation
2 participants