Skip to content
This repository has been archived by the owner on Dec 20, 2023. It is now read-only.

imp: deprecate ethereum-ledger-go components #8

Merged
merged 42 commits into from
Dec 8, 2022
Merged

imp: deprecate ethereum-ledger-go components #8

merged 42 commits into from
Dec 8, 2022

Conversation

fedekunze
Copy link
Contributor

No description provided.

@fedekunze fedekunze marked this pull request as ready for review December 4, 2022 16:01
@fedekunze fedekunze requested a review from a team as a code owner December 4, 2022 16:01
github-advanced-security[bot]
github-advanced-security bot found potential problems Dec 4, 2022
View reviewed changes
Copy link

@github-advanced-security github-advanced-security bot left a comment

Choose a reason for hiding this comment

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

gosec found more than 10 potential problems in the proposed changes. Check the Files changed tab for more details.

github-advanced-security[bot]
github-advanced-security bot found potential problems Dec 4, 2022
View reviewed changes
usbwallet/hub.go Fixed Show fixed Hide fixed
usbwallet/wallet.go

w.healthQuit = make(chan chan error)

go w.heartbeat()

Check notice

Code scanning / CodeQL

Spawning a Go routine

Spawning a Go routine may be a possible source of non-determinism
usbwallet/hub.go Fixed Show fixed Hide fixed
GAtom22
GAtom22 reviewed Dec 5, 2022
View reviewed changes
Copy link
Contributor

@GAtom22 GAtom22 left a comment

Choose a reason for hiding this comment

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

Great work! LGTM! Left a few comments

ledger.go Outdated Show resolved Hide resolved
ledger.go Outdated Show resolved Hide resolved
tests/integration_test.go Outdated Show resolved Hide resolved
tests/integration_test.go Outdated Show resolved Hide resolved
tests/integration_test.go Outdated Show resolved Hide resolved
usbwallet/ledger.go Show resolved Hide resolved
github-advanced-security[bot]
github-advanced-security bot found potential problems Dec 5, 2022
View reviewed changes
usbwallet/ledger.go Fixed Show fixed Hide fixed
usbwallet/ledger.go Fixed Show fixed Hide fixed
usbwallet/ledger.go Fixed Show fixed Hide fixed
usbwallet/ledger.go Fixed Show fixed Hide fixed
usbwallet/ledger.go Fixed Show fixed Hide fixed
usbwallet/ledger.go Fixed Show fixed Hide fixed
usbwallet/ledger.go Fixed Show fixed Hide fixed
usbwallet/ledger.go Fixed Show fixed Hide fixed
usbwallet/wallet.go Fixed Show fixed Hide fixed
usbwallet/wallet.go Fixed Show fixed Hide fixed
github-advanced-security[bot]
github-advanced-security bot found potential problems Dec 5, 2022
View reviewed changes
usbwallet/hub.go
"time"

//#nosec -- runtime is listed as a potential source for non-determinism, but we use it only for checking the OS
"runtime"

Check notice

Code scanning / CodeQL

Sensitive package import

Certain system packages contain functions which may be a possible source of non-determinism
github-advanced-security[bot]
github-advanced-security bot found potential problems Dec 7, 2022
View reviewed changes
usbwallet/wallet.go Fixed Show fixed Hide fixed
usbwallet/wallet.go Fixed Show fixed Hide fixed
MalteHerrmann and others added 2 commits December 7, 2022 13:00
@MalteHerrmann
use correct gosec codes for ignoring unhandled errors
4481e5e
@4rgon4ut @Vvaradinov @fedekunze
tests refactor (#10)
4d2e45a 
Co-authored-by: Vladislav Varadinov <[email protected]>
Co-authored-by: Federico Kunze Küllmer <[email protected]>
@fedekunze fedekunze dismissed 0a1c’s stale review December 7, 2022 22:09

addressed comments

fedekunze
fedekunze commented Dec 7, 2022
View reviewed changes
.github/workflows/test.yml Outdated Show resolved Hide resolved
fedekunze
fedekunze commented Dec 7, 2022
View reviewed changes
.github/workflows/test.yml Outdated Show resolved Hide resolved
github-advanced-security[bot]
github-advanced-security bot found potential problems Dec 8, 2022
View reviewed changes
usbwallet/ledger.go Fixed Show fixed Hide fixed
MalteHerrmann
MalteHerrmann reviewed Dec 8, 2022
View reviewed changes
usbwallet/ledger.go Outdated Show resolved Hide resolved
ledger/ledger_test.go Outdated Show resolved Hide resolved
Vvaradinov
Vvaradinov approved these changes Dec 8, 2022
View reviewed changes
Copy link
Contributor

@Vvaradinov Vvaradinov left a comment

Choose a reason for hiding this comment

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

LGTM. Made one final cleanup commit for the unused fields in the test suite setup.

GAtom22
GAtom22 approved these changes Dec 8, 2022
View reviewed changes
Copy link
Contributor

@GAtom22 GAtom22 left a comment

Choose a reason for hiding this comment

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

Great work! LGTM! Left only one comment

ledger/ledger_test.go Show resolved Hide resolved
@fedekunze fedekunze merged commit 1dfedc2 into main Dec 8, 2022
@fedekunze fedekunze deleted the fedekunze/merge-ledger branch December 8, 2022 14:56
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@Vvaradinov Vvaradinov Vvaradinov approved these changes

@MalteHerrmann MalteHerrmann MalteHerrmann left review comments

@4rgon4ut 4rgon4ut 4rgon4ut left review comments

@GAtom22 GAtom22 GAtom22 approved these changes

@danburck danburck Awaiting requested review from danburck

@facs95 facs95 Awaiting requested review from facs95

@0a1c 0a1c Awaiting requested review from 0a1c

Assignees
No one assigned
Labels
Type: Build Type: CI
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@fedekunze @Vvaradinov @0a1c @MalteHerrmann @GAtom22 @4rgon4ut