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

Only pass coin spends to get_conditions_from_spendbundle #643

Closed
wants to merge 16 commits into from
Closed

Only pass coin spends to get_conditions_from_spendbundle #643

wants to merge 16 commits into from

Conversation

Rigidity
Copy link
Contributor

@Rigidity Rigidity commented Aug 5, 2024

No description provided.

matt-o-how and others added 16 commits August 2, 2024 17:33
@matt-o-how
initial commit
1270f1d
@matt-o-how
add to lib
9ec59d9
@matt-o-how
add api info for pybindings
83398d9
@matt-o-how
cargo fmt
ce15eec
@matt-o-how
use uncached tree_hash()
0b73cb3
@matt-o-how
fmt
46de45e
@matt-o-how
add test for get_conditions_from_spendbundle()
54ff4d3
@matt-o-how
add regression test for get_flags
7546def
@matt-o-how
use underscored numbers for readability
907cd19
@arvidn
make get_conditions_from_spendbundle() return a non-owned SpendBundle…
b143770 
…Conditions (to enable reusing existing test facilities). Extend the tests to cover the block-generator test cases
@matt-o-how
paramterise test and use exact fork values
2613ac4
@matt-o-how
Merge pull request #640 from Chia-Network/tests-for-get_conditions_fr…
049430f 
…om_spendbundle

Extend the tests to cover the block-generator test cases
@matt-o-how
use named flags for case definitions
296f78c
@matt-o-how
remove unnecessary 0
8452eb3
@matt-o-how
fix typestub incorrect name
01cfdff
@Rigidity
Only pass coin spends to get_conditions_from_spendbundle
81f1112
@Rigidity Rigidity requested review from arvidn and matt-o-how August 5, 2024 17:42
arvidn
arvidn previously approved these changes Aug 6, 2024
View reviewed changes
Copy link
Contributor

@arvidn arvidn left a comment

Choose a reason for hiding this comment

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

The only reason I can think of to not do this is if we're planning to make get_conditions_from_spendbundle() validate the signature in the future

@Rigidity
Copy link
Contributor Author

Rigidity commented Aug 6, 2024

The only reason I can think of to not do this is if we're planning to make get_conditions_from_spendbundle() validate the signature in the future

Personally it's preferable to keep them separate since you often want to validate transactions, compute the cost, or unit test them without actually having valid signatures

Maybe it could be a flag though

Base automatically changed from get_flags_and_get_conditions to main August 6, 2024 11:11
@arvidn arvidn dismissed their stale review August 6, 2024 11:11

The base branch was changed.

@arvidn
Copy link
Contributor

arvidn commented Aug 9, 2024

Personally it's preferable to keep them separate since you often want to validate transactions, compute the cost, or unit test them without actually having valid signatures

Yeah, there are quite a lot of places where we trust the block to be valid (e.g. if we find it in our DB) but we just want to run it to get at the conditions. Then we don't want to also validate the signatures.

(this needs a rebase now btw)

@Rigidity
Copy link
Contributor Author

Closing for now

@Rigidity Rigidity closed this Aug 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@arvidn arvidn arvidn left review comments

@matt-o-how matt-o-how Awaiting requested review from matt-o-how

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Rigidity @arvidn @matt-o-how