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

clean-up(trusted,getter calls): remove raw payload signature support #2637

Merged
merged 13 commits into from
Apr 9, 2024
Merged

clean-up(trusted,getter calls): remove raw payload signature support #2637

merged 13 commits into from
Apr 9, 2024

Conversation

jonalvarezz
Copy link
Contributor

@jonalvarezz jonalvarezz commented Apr 3, 2024 •
edited

Context

Raw signatures isn't really used and the shorter hashed version should be preffered instead.

This pull request remove the raw payload checks support and rework ts-tests to hash the payload or the prefix + hashed payload combo.

TODO

  • drop raw payload verification support
  • update ts-tests
  • ts-tests: add prefix + hashed payload tests
  • test on IDH.

@linear Linear
Copy link

linear bot commented Apr 3, 2024

P-639 clean-up: remove raw payload signature verification

@jonalvarezz jonalvarezz marked this pull request as ready for review April 6, 2024 01:32
0xverin
0xverin reviewed Apr 8, 2024
View reviewed changes
Copy link
Contributor

@0xverin 0xverin left a comment

Choose a reason for hiding this comment

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

I only checked the ts-tests part, overall it looks good. Thank you for adding detailed logs of stferror. In fact, I was planning to do this because we had a related discussion before on the channel. :p

tee-worker/ts-tests/integration-tests/common/di-utils.ts Show resolved Hide resolved
tee-worker/ts-tests/integration-tests/di_substrate_identity.test.ts Show resolved Hide resolved
Kailai-Wang
Kailai-Wang reviewed Apr 8, 2024
View reviewed changes
Copy link
Collaborator

@Kailai-Wang Kailai-Wang left a comment

Choose a reason for hiding this comment

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

Thank you - I'm good to merge it once CI passes : )

@jonalvarezz jonalvarezz requested a review from 0xverin April 8, 2024 15:25
@jonalvarezz jonalvarezz added the C0-breaking Breaking change that will cause existing client to break label Apr 9, 2024
@jonalvarezz jonalvarezz merged commit f656039 into dev Apr 9, 2024
25 of 26 checks passed
@jonalvarezz jonalvarezz deleted the p-639-clean-up-remove-raw-payload-signature-verification branch April 9, 2024 01:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Kailai-Wang Kailai-Wang Kailai-Wang approved these changes

@kziemianek kziemianek Awaiting requested review from kziemianek

@Traf333 Traf333 Awaiting requested review from Traf333

@0xverin 0xverin Awaiting requested review from 0xverin

Assignees
No one assigned
Labels
C0-breaking Breaking change that will cause existing client to break
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@jonalvarezz @Kailai-Wang @0xverin