Skip to content

Eth and personal sign - #29

Merged
estebanmino merged 16 commits into
masterfrom
eth-personal-sign
Nov 30, 2018
Merged

Eth and personal sign#29
estebanmino merged 16 commits into
masterfrom
eth-personal-sign

Conversation

@estebanmino

Copy link
Copy Markdown
Contributor

This PR is mostly a migration to ts of this to have both eth_sign and personal_sign as proposed here MetaMask/metamask-extension#4117

It also exposes signPersonalMessage from eth-keyring-controller

@brunobar79brunobar79 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Minor comments but this is solid stuff @estebanmino 💯

import { EventEmitter } from 'events';
import BaseController, { BaseConfig, BaseState } from './BaseController';
import { validatePersonalSignMessageData, normalizeMessageData } from './util';
const random = require('uuid/v1');

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

import ?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

honestly I didn't question this, TransactionController uses the same and imports it in the same way

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You're right. Other files are also using require. Maybe we should standardize? cc: @bitpshr

Comment threadsrc/PersonalMessageManager.ts Outdated
/**
* @type Message
*
* Represents, and contains data about, an 'personal_sign' type signature request.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

"Represents, and contains data about a 'personal_sign' type signature request."

Comment threadsrc/PersonalMessageManager.ts Outdated
* @property id - An id to track and identify the message object
* @property messageParams - The parameters to pass to the personal_sign method once the signature request is approved
* @property type - The json-prc signing method for which a signature request has been made.
* A 'Message' with always have a 'personal_sign' type

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

"A 'Message' which always has..."

Comment threadsrc/PersonalMessageManager.ts Outdated
/**
* @type MessageParamsMetamask
*
* Represents, the parameters to pass to the personal_sign method once the signature request is approved

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Represents the (remove comma)

*
*/
getUnapprovedMessages() {
return this.messages

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ES6 + Functional programming + TypeScript = 💣 🔥

@codecov-io

codecov-io commented Nov 28, 2018

Copy link
Copy Markdown

Codecov Report

Merging #29 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@ Coverage Diff @@## master #29 +/- ##
======================================
Coverage 100% 100% ======================================
Files 18 19 +1 Lines 889 989 +100 Branches 97 107 +10 ======================================
+ Hits 889 989 +100
Impacted FilesCoverage Δ
src/PersonalMessageManager.ts100% <100%> (ø)
src/util.ts100% <100%> (ø)⬆️
src/CurrencyRateController.ts100% <0%> (ø)⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e2098be...01a1b7b. Read the comment docs.

@estebanmino
estebanmino merged commit 12e3f7b into masterNov 30, 2018
@estebanmino
estebanmino deleted the eth-personal-sign branch November 30, 2018 01:21
mcmire pushed a commit to mcmire/core that referenced this pull request Jul 17, 2023
kanthesha pushed a commit that referenced this pull request Sep 19, 2023
MajorLift added a commit that referenced this pull request Sep 29, 2023
* 2.2.0
* Update CHANGELOG.md
* Add license update as entry to CHANGELOG.md
---------
Co-authored-by: github-actions <github-actions@github.com>
Co-authored-by: Jongsun Suh <34228073+MajorLift@users.noreply.github.com>
Mrtenz pushed a commit that referenced this pull request Oct 16, 2025
Mrtenz pushed a commit that referenced this pull request Oct 16, 2025
Sign up for freeto 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

@estebanmino@codecov-io@brunobar79