Skip to content
This repository has been archived by the owner on Jun 11, 2024. It is now read-only.

Add signing function for non protocol message #429

Merged
merged 6 commits into from
Jul 27, 2023

Conversation

AndreasKendziorra
Copy link
Contributor

@AndreasKendziorra AndreasKendziorra commented Jul 25, 2023

Fixes #427

Copy link
Contributor

@sergeyshemyakov sergeyshemyakov left a comment

Choose a reason for hiding this comment

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

Just one suggestion for the rationale.

proposals/lip-0062.md Show resolved Hide resolved
proposals/lip-0062.md Outdated Show resolved Hide resolved
Copy link
Contributor

@AnanyaSshri AnanyaSshri left a comment

Choose a reason for hiding this comment

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

Approved, just a minor suggestion.

Copy link
Contributor

@sergeyshemyakov sergeyshemyakov left a comment

Choose a reason for hiding this comment

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

Looks good to me now.

@eniolam1000752 eniolam1000752 self-requested a review July 26, 2023 13:14
A non-protocol related message `message` can be signed by the function `signNonProtocolEd25519`. The resulting signature can be verified by `verifyNonProtocolEd25519`. This scheme should be used for the _sign message_ feature in Lisk Desktop. Note that the tag `MESSAGE_TAG_NON_PROTOCOL_MESSAGE` is defined in [LIP 0037](https://github.com/LiskHQ/lips/blob/main/proposals/lip-0037.md#message-tags-1).

```python
def signNonProtocolEd25519(sk: bytes, message: bytes) -> bytes:
Copy link
Collaborator

Choose a reason for hiding this comment

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

maybe not necessary to align, but for the SDK we have functions like

signMessageWithPrivateKey
verifyMessageWithPublicKey

so and it's used like ed.signMessageWithPrivateKey. I would prefer to keep those names

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see the reason to keep the names in the SDK and it's fine for me. Would be good to haven some comments then. I'm also open to new names for the LIP as I'm not satisfied with them as well. But I didn't have a better idea. But I would like to avoid signMessageWithPrivateKey as it is really not descriptive.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

An alternative could be signOffchainDataEd25519.

@shuse2
Copy link
Collaborator

shuse2 commented Jul 26, 2023

Created corresponding issue on SDK: LiskArchive/lisk-sdk#8775

Copy link
Contributor

@janhack janhack left a comment

Choose a reason for hiding this comment

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

Editorial approval

@janhack janhack merged commit f1a9863 into main Jul 27, 2023
@janhack janhack deleted the add_signing_for_non-protocol_message branch July 27, 2023 13:01
@ManuGowda ManuGowda removed their request for review August 2, 2023 09:11
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The usage of double pre-hashing and single pre-hashing enables collisions
6 participants