Skip to content

Conversation

@sreblock
Copy link

Description

Updates from these pull requests

  {
    "createdAt": "2023-03-15T16:14:13Z",
    "headRefName": "update-ethereumjs-deps",
    "isCrossRepository": false,
    "mergedAt": "2023-03-15T19:55:11Z",
    "title": "update ethereumjs dependencies",
    "url": "https://github.com/MetaMask/eth-ledger-bridge-keyring/pull/177"
  }
]```

@sreblock sreblock added the automated PR made by an automation label Mar 16, 2023
@sreblock sreblock self-assigned this Mar 16, 2023
@sreblock sreblock requested a review from julianariel March 16, 2023 12:20
brad-decker and others added 26 commits March 20, 2023 12:17
Co-authored-by: Mark Stacey <[email protected]>
Co-authored-by: brad-decker <[email protected]>
* chore: add tsconfig and typedoc

* feat: add typed LedgerBridgeKeyring

* test: typed tests for ledger keyring

* ci: add build step to circleci

* ci: fix build step

* chore: update linting pipeline

* fix: edit package.json main types and files values

* refactor: use @ledgerhq/hw-app-eth to get types

* fix: review refactor

* refactor: use sharp notation on private methods

* refactor: use sharp notation on _sendMessage

* refactor: use sharp notation for _eventListener

* refactor: change destroy tests

* refator: remove unused private functions

* refactor: apply review suggestions

* refactor: @mcmire review

* chore: add comments

* refactor: update eslint packages

* fix: Event not defined in ci

* fix: eslint version mismatch and fix lint

* refactor: networkApiUrls as enum

* fix: use prettierrc from module template

* refactor: isOldStyleEthereumjsTx check

* refactor: apply @andrewpeters9 suggestion

* refactor: @Gudahtt suggestions

* refactor: remove useless promise catch

* refactor: use eslint-config-browser

* fix: use eslint-config-nodejs on tests
* ci: remove circleci in favor of gh action workflow

* chore: standardizations from module template

* chore: run prettier on yml

* ci: add typescript build and publish docs step

* test: empty commit to re-run workflows

* fix: changelog

* ci: remove publish doc related steps

* fix: malformed changelog links

* fix: repository url in package.json
* test: migrate to jest

* docs: edit testing paragraph
* v0.16.0

* fix changelog

* rewording

* rewording

* bump to 1.0.0

* typo
* 1.0.0

* changelog

---------

Co-authored-by: github-actions <[email protected]>
Co-authored-by: Bernardo Garces Chapero <[email protected]>
Bumps [semver](https://github.com/npm/node-semver) from 6.3.0 to 6.3.1.
- [Release notes](https://github.com/npm/node-semver/releases)
- [Changelog](https://github.com/npm/node-semver/blob/v6.3.1/CHANGELOG.md)
- [Commits](npm/node-semver@v6.3.0...v6.3.1)

---
updated-dependencies:
- dependency-name: semver
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
* deps: @metamask/eth-sig-util@^2.0.0->^6.0.1
* devDeps: add missing @ledgerhq/types implicit peerDeps
* lint:fix
* test: fix jest.spyOn of @metamask/eth-sig-util by mocking es module

---------

Co-authored-by: legobt <[email protected]>
Bumps [@metamask/utils](https://github.com/MetaMask/utils) from 5.0.0 to 5.0.2.
- [Release notes](https://github.com/MetaMask/utils/releases)
- [Changelog](https://github.com/MetaMask/utils/blob/main/CHANGELOG.md)
- [Commits](MetaMask/utils@v5.0.0...v5.0.2)

---
updated-dependencies:
- dependency-name: "@metamask/utils"
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

---------

Co-authored-by: github-actions <[email protected]>
Co-authored-by: legobt <[email protected]>
- Remove support for major node versions 14,15,17,19
- Set nodejs version 18 in .nvmrc
- ci: Run on Node.js v16,v18,v20 (default v18) instead of
  v14,v16,v18,v19 (default v16)
github-actions bot and others added 26 commits November 6, 2023 12:11
* 2.0.0

* udate changelog

---------

Co-authored-by: github-actions <[email protected]>
Co-authored-by: legobt <[email protected]>
… modular types (#207)

## Motivation

`IFrameMessageResponse` is currently defined with a generic parameter `TAction`. This parameter doesn't usefully narrow or widen the type. 
- The `action: TAction` property appears to *widen* `IFrameMessageResponse` to accept any action type, but `TAction` is also constrained as a subtype of `IFrameMessageActions`.
- The `TAction` generic param appears to *narrow* `IFrameMessageResponse` to select individual union members, but it interferes with the inference-based type narrowing we can get if `IFrameMessageResponse` is an exhaustively enumerated discriminated union. 

This causes two problems:

1. Type errors on assignment operations that should be valid.
- e.g. [`as` type casting](https://github.com/MetaMask/eth-ledger-bridge-keyring/pull/207/files#diff-c901ebd8641651c9156a86894574a6d9567f2ae79495de0edff2cbef8ae958f0L302-L304) is necessary here because supertype is being assigned to a subtype i.e. `(response: IFrameMessageResponse<TAction>) => void` to `(response: IFrameMessageResponse<IFrameMessageActions>) => void`.
  - In general, `(x: SuperType) => T` is a *subtype* of `(x: SubType) => T`.
- **This error indicates an underlying issue with the typing and shouldn't be suppressed.**
- Removing `TAction` puts `(response: SomeIFrameMessageResponse) => void` on the LHS (assignee, supertype) and `(response: IFrameMessageResponse) => void` on the RHS (assigned, subtype), resolving this logical error.

2. `TAction` is also silencing type errors we should be getting from type narrowing based on `action` value. 
- e.g. Some union members of `IFrameMessageResponse` do not include a `success`, `error`, `payload`, or `payload.error` property, but because of `TAction`, TypeScript doesn't alert us that we should be performing `in` checks on them in addition to null checks. 
- This can cause unexpected failure at runtime, especially from the destructuring assignments that are being used.

Constraining `IFrameMessageResponse['action']` to `IFrameMessageActions` both resolves these issues and guides us towards writing type-safe logic about these actions that conform to their specific type signatures. It appears that this was the original intention of writing `IFrameMessageActions` as a discriminated union instead of a wider type encompassing all possible actions.

## Changes

- **BREAKING** Narrows `IFrameMessageResponse` type definition

## Explanation

- To resolve these issues, This PR makes `IFrameMessageResponse` non-generic and removes `as` casting.
- For improved readability and maintainability, `IFrameMessageResponse` is also refactored into a union of named types.
- A `IFrameMessageResponseBase` type is defined for modularity and better visibility of `IFrameMessageResponse` types with atypical shapes.

## References

- Closes #208
- More info on function subtyping: MetaMask/core#1890 (comment)
---------

Co-authored-by: github-actions <[email protected]>
Co-authored-by: legobt <[email protected]>
- BREAKING: `LedgerIframeBridge` class constructor now takes an options object with `bridgeUrl`. 
- BREAKING: `LedgerBridge` `init` function now takes no parameters.
- BREAKING: `LedgerBridgeKeyringOptions` no longer contain `bridgeUrl`. 
- BREAKING: `LedgerBridge` interface is now parameterized over its option type
-  Add `getOptions` and `setOptions` methods to `LedgerBridge` interface
---------

Co-authored-by: github-actions <[email protected]>
Co-authored-by: legobt <[email protected]>
* Enabling security code scanner

* Update security code scanner file
* feat: add mobile keyring bridge

* chore: update version number

* feat: upgrade @ethereumjs/tx library to match the same version used in TransactionController 8.0.1 to fix all transaction broken issue.

Fix the signPersonalMessage in ledger always return 0x0 in v value in test-dapp

* feat: upgrade ledgerHQ library to latest to resolve vulnerablity report from socket-security

* Feat/add mobile keyring bridge flat version (#3)

* feat: this is a backup of flat directory version of eth-ledger-bridge-keyring to successfully solve the metamask-mobile not able to resovle all the module under the subdirectory during build process.

* feat: improve the unit tests coverage for some API.

* feat: run `yarn lint:fix` and update the correct import in index.ts file.

* feat: add unit tests coverage

* feat: fix lint error in test file.

* feat: Remove unit tests which failed.

* feat: Add extra test to cover more lines in ledger-keyring.ts

* feat: Add yarn lint:fix

* feat: Increase the threshold coverage to pass the pipeline.

* feat: revert the version of package..

* Update src/ledger-transport-middleware.ts

Co-authored-by: Gustavo Antunes <[email protected]>

* Update src/ledger-mobile-bridge.test.ts

Co-authored-by: Gustavo Antunes <[email protected]>

* Update src/ledger-mobile-bridge.test.ts

Co-authored-by: Gustavo Antunes <[email protected]>

* Update src/ledger-mobile-bridge.test.ts

Co-authored-by: Gustavo Antunes <[email protected]>

* Apply suggestions from code review

Co-authored-by: Gustavo Antunes <[email protected]>

* feat: remove the comment code in `prepack.sh`

* feat: change the jsDoc for deviceSignMessage method.

* feat: fix the lint issue.

* Apply suggestions from code review

Co-authored-by: Gustavo Antunes <[email protected]>

* Apply suggestions from code review

Co-authored-by: Gustavo Antunes <[email protected]>

* feat: change setAccountToUnlock method signature to enforce index is number type.

* feat: change setAccountToUnlock method signature to enforce index is number type.

* feat: commit the new coverageThreshold to pass the test.

---------

Co-authored-by: Xiaoming Wang <[email protected]>
Co-authored-by: Gustavo Antunes <[email protected]>
* 4.0.0

* chore: update README

* chore: update README

* chore: update README

---------

Co-authored-by: github-actions <[email protected]>
Co-authored-by: gantunesr <[email protected]>
Currently, in iOS devices a disconnect error will be thrown when trying to reset the transport object. The reason for this is because the ETH app hasn't been recreated with the new transport object, even though the transport has the same properties as the previous one.

Note: We do not see this in Android devices, the setup and communication will work correctly without the need to reset the transport object.

Co-authored-by: Gustavo Antunes <[email protected]>
* fix: Currently ledger keyring throw `unknown error` when user didn't turn on `eth` app in ledger. this code change provide better error message to indicate what happen.

* fix: Currently ledger keyring throw `unknown error` when user lock ledger or didn't turn on `eth` app in ledger in MM extension. this code change provide better error message to indicate what happen.

* fix: Currently ledger keyring throw `unknown error` when user lock ledger or didn't turn on `eth` app in ledger in MM extension. this code change provide better error message to indicate what happen.

* feat: Remove duplicate tests.

* feat: Change the error message to `Ledger Ethereum app closed. Open it to unlock.`

* feat: Change the error message to `Ledger Ethereum app closed. Open it to unlock.`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

automated PR made by an automation

Projects

None yet

Development

Successfully merging this pull request may close these issues.