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

Integrate message signing #28

Merged
merged 6 commits into from
Jul 15, 2021
Merged

Conversation

ccorcoveanu
Copy link
Contributor

Code from this PR is in great part integrated thanks to @alexandru-g who opened this PR in the old repo:
https://github.com/ElrondNetwork/elrond-sdk/pull/215/files

There are some modifications though, mostly related to the architecture following the same pattern as our transaction signers. Also all DAPP providers (except HWWallet) throw exceptions on signMessage because the prepended message should be enforced by the provider itself.

@bogdan-rosianu bogdan-rosianu self-requested a review July 15, 2021 10:28
src/interface.ts Outdated
@@ -124,6 +129,20 @@ export interface ISignable {
applySignature(signature: Signature, signedBy: Address): void;
}

/**
* Interface that defines a signed and verifieble object
Copy link
Contributor

Choose a reason for hiding this comment

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

verifiable

@@ -234,6 +234,10 @@ export class Transaction implements ISignable {
serializeForSigning(signedBy: Address): Buffer {
// TODO: for appropriate tx.version, interpret tx.options accordingly and sign using the content / data hash
let plain = this.toPlainObject(signedBy);
// Make sure we never sign the transaction with another signature set up (useful when using the same method for verification)
if (plain.signature) {
Copy link
Contributor

Choose a reason for hiding this comment

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

shouldn't this be part of toPlainObject() ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no, we should be able to convert toPlainObject while still keeping the signature. The sig removal should only be done while verifying the signature

@ccorcoveanu ccorcoveanu merged commit ebbf347 into development Jul 15, 2021
@ccorcoveanu ccorcoveanu deleted the integrate-message-signing branch July 15, 2021 11:56
andreibancioiu added a commit that referenced this pull request Sep 25, 2024
Broadcast transactions, in bulk
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.

3 participants