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

Add Keyring types #74

Merged
merged 32 commits into from
Feb 18, 2023
Merged

Add Keyring types #74

merged 32 commits into from
Feb 18, 2023

Conversation

Gudahtt
Copy link
Member

@Gudahtt Gudahtt commented Jan 24, 2023

WIP; description to be added later.

This is a port of this old draft PR: https://github.com/MetaMask/types/pull/5/files

@Gudahtt
Add Keyring type
6eea59f 
WIP; description to be added later.
Gudahtt
Gudahtt commented Jan 24, 2023
View reviewed changes
src/encryption.ts Outdated Show resolved Hide resolved
Gudahtt
Gudahtt commented Jan 24, 2023
View reviewed changes
src/keyring.ts Outdated
@@ -0,0 +1,250 @@
import type { Transaction } from '@ethereumjs/tx';
Copy link
Member Author

Choose a reason for hiding this comment

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

TODO: Find a replacement Transaction type that doesn't tie us to this specific library.

Copy link
Member Author

Choose a reason for hiding this comment

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

Maybe this is what we need: https://github.com/ethereumjs/ethereumjs-monorepo/blob/7d7b0ad0915b560093a7e1ac5802432e40f436f5/packages/tx/src/types.ts#L143

@Gudahtt Gudahtt mentioned this pull request Jan 24, 2023
Closed
35 tasks
@Gudahtt Gudahtt assigned mikesposito and unassigned mikesposito Feb 1, 2023
gantunesr
gantunesr reviewed Feb 13, 2023
View reviewed changes
src/transaction-types.ts Outdated Show resolved Hide resolved
src/transaction-types.ts Outdated Show resolved Hide resolved
Gudahtt
Gudahtt commented Feb 14, 2023
View reviewed changes
src/transaction-types.ts Outdated Show resolved Hide resolved
src/transaction-types.ts Outdated Show resolved Hide resolved
@gantunesr gantunesr marked this pull request as ready for review February 16, 2023 20:12
@gantunesr gantunesr requested a review from a team as a code owner February 16, 2023 20:12
Mrtenz
Mrtenz requested changes Feb 17, 2023
View reviewed changes
src/transaction-types.ts Outdated Show resolved Hide resolved
src/transaction-types.ts Outdated Show resolved Hide resolved
src/transaction-types.ts Outdated Show resolved Hide resolved
src/transaction-types.ts Outdated Show resolved Hide resolved
src/transaction-types.ts Outdated Show resolved Hide resolved
src/transaction-types.ts Outdated Show resolved Hide resolved
Mrtenz
Mrtenz requested changes Feb 17, 2023
View reviewed changes
Copy link
Member

@Mrtenz Mrtenz left a comment

Choose a reason for hiding this comment

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

We have utils for working with bytes, which can do almost everything that Buffer can do, so I would strongly prefer using that.

src/transaction-types.ts Outdated Show resolved Hide resolved
src/transaction-types.ts Outdated Show resolved Hide resolved
src/transaction-types.ts Outdated Show resolved Hide resolved
src/transaction-types.ts Outdated Show resolved Hide resolved
src/transaction-types.ts Outdated Show resolved Hide resolved
src/transaction-types.ts Outdated Show resolved Hide resolved
src/transaction-types.ts Outdated Show resolved Hide resolved
src/transaction-types.ts Outdated Show resolved Hide resolved
src/transaction-types.ts Outdated Show resolved Hide resolved
src/transaction-types.ts Outdated Show resolved Hide resolved
gantunesr and others added 11 commits February 17, 2023 13:41
@gantunesr @Mrtenz
Update data type
3780038 
Co-authored-by: Maarten Zuidhoorn <maarten@zuidhoorn.com>
@gantunesr @Mrtenz
Update data type
2f383ae 
Co-authored-by: Maarten Zuidhoorn <maarten@zuidhoorn.com>
@gantunesr @Mrtenz
Update data type
4e5d65b 
Co-authored-by: Maarten Zuidhoorn <maarten@zuidhoorn.com>
@gantunesr @Mrtenz
Update data type
2144cd6 
Co-authored-by: Maarten Zuidhoorn <maarten@zuidhoorn.com>
@gantunesr @Mrtenz
Update data type
05b677a 
Co-authored-by: Maarten Zuidhoorn <maarten@zuidhoorn.com>
@gantunesr @Mrtenz
Update data type
b5fdde1 
Co-authored-by: Maarten Zuidhoorn <maarten@zuidhoorn.com>
@gantunesr @Mrtenz
Update data type
9832f6d 
Co-authored-by: Maarten Zuidhoorn <maarten@zuidhoorn.com>
@gantunesr @Mrtenz
Update data type
e16ff86 
Co-authored-by: Maarten Zuidhoorn <maarten@zuidhoorn.com>
@gantunesr @Mrtenz
Update data type
a10996d 
Co-authored-by: Maarten Zuidhoorn <maarten@zuidhoorn.com>
@gantunesr @Mrtenz
Update data type
6f1d569 
Co-authored-by: Maarten Zuidhoorn <maarten@zuidhoorn.com>
@gantunesr @Mrtenz
Update data type
c8cfa1e 
Co-authored-by: Maarten Zuidhoorn <maarten@zuidhoorn.com>
Mrtenz
Mrtenz approved these changes Feb 17, 2023
View reviewed changes
Copy link
Member

@Mrtenz Mrtenz left a comment

Choose a reason for hiding this comment

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

This looks great!

Mrtenz
Mrtenz requested changes Feb 17, 2023
View reviewed changes
src/transaction-types.ts Outdated Show resolved Hide resolved
@Gudahtt
Copy link
Member Author

Gudahtt commented Feb 18, 2023

LGTM! (it won't let me approve because I opened it)

@gantunesr gantunesr merged commit d4fe393 into main Feb 18, 2023
@gantunesr gantunesr deleted the add-keyring-type branch February 18, 2023 23:08
@gantunesr gantunesr mentioned this pull request Feb 20, 2023
Merged
@mikesposito mikesposito mentioned this pull request Mar 14, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gantunesr gantunesr gantunesr left review comments

@Mrtenz Mrtenz Mrtenz approved these changes

@montelaidev montelaidev Awaiting requested review from montelaidev

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@Gudahtt @Mrtenz @gantunesr @montelaidev @mikesposito