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

feat(sdk-coin-avaxp): add avaxp new SDK #4361

Merged
merged 5 commits into from
Mar 26, 2024
Merged

feat(sdk-coin-avaxp): add avaxp new SDK #4361

merged 5 commits into from
Mar 26, 2024

Conversation

noel-bitgo
Copy link
Contributor

CR-1073

TICKET: CR-1073

@socket-security Socket Security
Copy link

socket-security bot commented Mar 22, 2024 •
edited
Loading

New and removed dependencies detected. Learn more about Socket for GitHub ↗︎

Package New capabilities Transitives Size Publisher
npm/@nodelib/[email protected] filesystem 0 11.8 kB mrmlnc
npm/[email protected] None 0 3.17 kB sindresorhus
npm/[email protected] None 0 3.57 kB sindresorhus
npm/[email protected] None 0 33.9 kB kriskowal
npm/[email protected] None 0 9.62 kB feross
npm/[email protected] None 0 11.7 kB sindresorhus
npm/[email protected] None 0 10.5 kB dougwilson
npm/[email protected] None 0 14.5 kB andyburke
npm/[email protected] environment, eval 0 27.1 kB dougwilson
npm/[email protected] None 0 369 kB kpdecker
npm/[email protected] None 0 12.3 kB ljharb
npm/[email protected] None 0 314 kB ariya
npm/[email protected] None 0 23.5 kB ljharb
npm/[email protected] None 0 39.7 kB matteo.collina
npm/[email protected] None 0 16.7 kB ljharb
npm/[email protected] None 0 4.72 kB stefanpenner
npm/[email protected] None 0 12.8 kB ljharb
npm/[email protected] None 0 20.6 kB ljharb
npm/[email protected] None 0 124 kB mathias
npm/[email protected] None 0 7.72 kB lukeed
npm/[email protected] None 0 9.3 kB isaacs
npm/[email protected] None 0 28.9 kB ljharb
npm/[email protected] None 0 12.7 kB isaacs
npm/[email protected] None 0 235 kB jordanbtucker
npm/[email protected] None 0 1.41 MB bnjmnt4n
npm/[email protected] None 0 8.9 kB zensh
npm/[email protected] network 0 5.29 kB dougwilson
npm/[email protected] None 0 206 kB dougwilson
npm/[email protected] None 0 27.4 kB dougwilson
npm/[email protected] None 0 9.22 kB jonschlinkert
npm/[email protected] None 0 5.49 kB sindresorhus
npm/[email protected] None 0 10.3 kB dougwilson
npm/[email protected] None 0 4.55 kB sindresorhus
npm/[email protected] None 0 90 kB mrmlnc
npm/[email protected] None 0 32.1 kB feross
npm/[email protected] None 0 42.3 kB chalker
npm/[email protected] None 0 6.96 kB sindresorhus
npm/[email protected] None 0 4.31 kB dougwilson
npm/[email protected] None 0 116 kB ctavan
npm/[email protected] None 0 8.75 kB dougwilson
npm/[email protected] None 0 14.8 kB isaacs

View full report↗︎

sol-mocha

This comment was marked as resolved.

Sign in to view

zahin-mohammad
zahin-mohammad requested changes Mar 25, 2024
View reviewed changes
Copy link
Contributor

@zahin-mohammad zahin-mohammad left a comment

Choose a reason for hiding this comment

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

Few comments:

  • Browser tests won't pass due to cypress
  • Changing min version to node 18 is a breaking change, you need to add the appropriate footer tag for this

I already did both in my node 16 PR, please rebate off of mine.
#4119

@noel-bitgo
Copy link
Contributor Author

Few comments:

  • Browser tests won't pass due to cypress

  • Changing min version to node 18 is a breaking change, you need to add the appropriate footer tag for this

I already did both in my node 16 PR, please rebate off of mine.

#4119

Amazing thanks!
Yeah this PR was just testing feasibility of using the new avaxp sdk on node18

@socket-security Socket Security
Copy link

socket-security bot commented Mar 25, 2024 •
edited
Loading

👍 Dependency issues cleared. Learn more about Socket for GitHub ↗︎

This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored.

View full report↗︎

@noel-bitgo noel-bitgo force-pushed the CR-1073 branch 2 times, most recently from 837b3c7 to acee038 Compare March 25, 2024 14:24
@CapnMigraine CapnMigraine force-pushed the CR-1073 branch 2 times, most recently from cb41794 to b7fdba2 Compare March 25, 2024 19:05
@noel-bitgo noel-bitgo marked this pull request as ready for review March 25, 2024 19:29
@noel-bitgo noel-bitgo requested review from a team as code owners March 25, 2024 19:29
@noel-bitgo noel-bitgo force-pushed the CR-1073 branch 3 times, most recently from 58ecbcf to 6d26127 Compare March 26, 2024 13:35
@noel-bitgo noel-bitgo marked this pull request as draft March 26, 2024 15:51
@andrew-scott-fischer
Copy link
Contributor

andrew-scott-fischer commented Mar 26, 2024 •
edited
Loading

Few comments:

  • Browser tests won't pass due to cypress

Looks like browser-tests did pass? Why wasn't that expected? @zahin-mohammad @noel-bitgo ?

@zahin-mohammad
Copy link
Contributor

zahin-mohammad commented Mar 26, 2024 •
edited
Loading

Few comments:

  • Browser tests won't pass due to cypress

Looks like browser-tests did pass? Why wasn't that expected? @zahin-mohammad @noel-bitgo ?

Honestly not 100% sure why, but upgrading cypress fixed it for me. https://bitgo.slack.com/archives/C010AEXLLCR/p1711229931703669

They work now because this PR is rebased off of my node 16 deprecation PR.

@andrew-scott-fischer
Copy link
Contributor

  • Changing min version to node 18 is a breaking change, you need to add the appropriate footer tag for this

@zahin-mohammad When you merge your PR to remove 16, update the readme too

@zahin-mohammad
Copy link
Contributor

  • Changing min version to node 18 is a breaking change, you need to add the appropriate footer tag for this

@zahin-mohammad When you merge your PR to remove 16, update the readme too

Ah must have missed it, good catch.

@zahin-mohammad
Copy link
Contributor

  • Changing min version to node 18 is a breaking change, you need to add the appropriate footer tag for this

@zahin-mohammad When you merge your PR to remove 16, update the readme too

https://github.com/BitGo/BitGoJS/pull/4371/files

@noel-bitgo noel-bitgo marked this pull request as ready for review March 26, 2024 17:54
@noel-bitgo noel-bitgo requested a review from a team as a code owner March 26, 2024 17:54
noel-bitgo and others added 5 commits March 26, 2024 11:10
@noel-bitgo @andrew-scott-fischer
feat(sdk-coin-avaxp): add new avaxp library
14d3767 
new AVAX library minimum node version is node 20.11.1
https://github.com/ava-labs/avalanchejs

this repo supports both node 18 and node 20

this PR uses a forked version of the avax library that supports node 18
https://www.npmjs.com/package/bitgo-aaron-avalanchejs?activeTab=readme

TICKET: CR-1073
@CapnMigraine @andrew-scott-fischer
feat(sdk-coin-avaxp): add skeleton for addPermissionLessValidator tx
08ad690 
TICKET: CR-1073
@andrew-scott-fischer
chore: update BitGoJS to use [email protected]
655126d 
Ticket: VL-1844
@andrew-scott-fischer
fix: ignore type-checks of error objects
82c9cae 
In TypeScript 4.4 error objects changed from `any` to `unknown`.
Rather than fix this, this change ignores it.

Ticket: VL-1844
@andrew-scott-fischer
fix: cast as unknown before recasting
abe6595 
Exising code cast one object as another; the type might be wrong
but I'm not a SME. Casting `as unknown` to suppress TypeScript
errors.

Ticket: VL-1844
@noel-bitgo noel-bitgo merged commit 4b6738b into master Mar 26, 2024
8 checks passed
@Ernel1997 Ernel1997 mentioned this pull request Oct 12, 2024
Open
@Matthelonianxl Matthelonianxl mentioned this pull request Oct 18, 2024
Open
@Ernel1997 Ernel1997 mentioned this pull request Oct 19, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@davidkaplanbitgo davidkaplanbitgo davidkaplanbitgo approved these changes

@ewangbitgo ewangbitgo ewangbitgo approved these changes

@YPAzevedo YPAzevedo YPAzevedo approved these changes

@zahin-mohammad zahin-mohammad zahin-mohammad approved these changes

@CapnMigraine CapnMigraine CapnMigraine approved these changes

@OttoAllmendinger OttoAllmendinger OttoAllmendinger approved these changes

@andrew-scott-fischer andrew-scott-fischer Awaiting requested review from andrew-scott-fischer

@DinshawKothari DinshawKothari Awaiting requested review from DinshawKothari

@sol-mocha sol-mocha Awaiting requested review from sol-mocha

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

Successfully merging this pull request may close these issues.
10 participants
@noel-bitgo @andrew-scott-fischer @zahin-mohammad @OttoAllmendinger @CapnMigraine @YPAzevedo @sol-mocha @ewangbitgo @davidkaplanbitgo @DinshawKothari