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

Update test vectors for ZIP 244 changes #61

Merged
merged 4 commits into from
Jan 7, 2022
Merged

Update test vectors for ZIP 244 changes #61

merged 4 commits into from
Jan 7, 2022

Conversation

str4d
Copy link
Contributor

@str4d str4d commented Jan 6, 2022

Closes #57.

Signatures for shielded inputs also commit to these new commitments,
meaning that for mixed transactions, shielded signatures are no longer
equivalent to signing the txid. This property remains for fully shielded
transactions.
@str4d str4d added this to the Core Sprint 2021-52 milestone Jan 6, 2022
Copy link
Contributor

@daira daira left a comment

Choose a reason for hiding this comment

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

utACK

Comment on lines +403 to +405
('amounts', '[i64; %d]' % len(t_inputs)),
('script_codes', {
'rust_type': '[Vec<u8>; %d]' % len(t_inputs),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Kris noted that this causes a bug in the output, because len(t_inputs) returns the length for the last test vector, but it varies across them. This should be:

Suggested change
('amounts', '[i64; %d]' % len(t_inputs)),
('script_codes', {
'rust_type': '[Vec<u8>; %d]' % len(t_inputs),
('amounts', 'Vec<i64>'),
('script_codes', {
'rust_type': 'Vec<Vec<u8>>',

but that won't work without more changes to the Rust output renderer, so instead I'm going to overhaul the latter after this PR.

@str4d str4d merged commit 6ee8ff7 into master Jan 7, 2022
@str4d str4d deleted the 57-zip244-changes branch January 7, 2022 11:57
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.

ZIP 244: Implement changes to bring its transparent semantics closer to BIP 341
2 participants