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.

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

refactor: upgrade stacks js, closes LEA-1989 #6076

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from

Conversation

fbwoolf
Copy link
Contributor

@fbwoolf fbwoolf commented Jan 21, 2025

Building Leather at commit 3a79248

Upgrades all stacks.js pkgs to 7.0.2.

Copy link

linear bot commented Jan 21, 2025

@fbwoolf fbwoolf requested a review from kyranjamie January 21, 2025 00:39
@fbwoolf
Copy link
Contributor Author

fbwoolf commented Jan 21, 2025

Dependent on changes installed from here: leather-io/mono#796

@fbwoolf
Copy link
Contributor Author

fbwoolf commented Jan 21, 2025

@kyranjamie I'm testing the changes here, but can you look over my changes to the stacks private and public key code changes? And, stacks.js removed decrypt and encrypt. I replaced them with decryptMnemonic and encryptMnemonic from @stacks/encryption, but I'm not sure if that is correct? There wasn't instruction in the migration doc. 🤔

@kyranjamie
Copy link
Collaborator

I replaced them with decryptMnemonic and encryptMnemonic from @stacks/encryption, but I'm not sure if that is correct? There wasn't instruction in the migration doc. 🤔

I'll get to testing this when I've fixed some tests for the returning txid issue. But re this, I'd make sure they're backwards compatible. Do these new methods decrypt payloads made before v7? Pretty sure they won't, so then we'll have to have some migration step, or bring over the old encryption methods from v6.

@fbwoolf
Copy link
Contributor Author

fbwoolf commented Jan 21, 2025

I'll get to testing this when I've fixed some tests for the returning txid issue. But re this, I'd make sure they're backwards compatible. Do these new methods decrypt payloads made before v7? Pretty sure they won't, so then we'll have to have some migration step, or bring over the old encryption methods from v6.

I tested going from dev > to this branch, creating a wallet > locking > then unlocking each way and did not hit any errors. Would love for you to test too though just to make sure works on your end. Maybe the change is ok? https://github.com/hirosystems/stacks.js/blob/main/.github/MIGRATION.md#triplesec

EDIT: This was the code removed with aliased methods:
https://www.npmjs.com/package/@stacks/wallet-sdk/v/6.15.0?activeTab=code

@fbwoolf
Copy link
Contributor Author

fbwoolf commented Jan 21, 2025

Something isn't quite right bc running into this restoring the test wallet. Balances are correct, but BNS name lookup is incorrect and appears to only restore one account. Looking into it...

Screenshot 2025-01-21 at 12 45 14 PM

EDIT: Loading, but a few BNS names are funky 🤔

Screenshot 2025-01-21 at 2 53 49 PM

EDIT: This appears to actually be a bug with BNS changes in the mono repo upgrade, so looking at fixing there.

@fbwoolf fbwoolf force-pushed the refactor/upgrade-stacks-js branch from ab265bb to 0c92d71 Compare January 22, 2025 19:51
@fbwoolf
Copy link
Contributor Author

fbwoolf commented Jan 22, 2025

@kyranjamie all seems to be working when I test now so opening up for your code review. Tests will be broken until I actually install mono repo changes, but hoping you will look over when can.

@fbwoolf fbwoolf marked this pull request as ready for review January 22, 2025 19:55
@fbwoolf fbwoolf force-pushed the refactor/upgrade-stacks-js branch 2 times, most recently from 29648bc to 9e917d1 Compare January 22, 2025 20:17
@kyranjamie
Copy link
Collaborator

Alright let's try getting this out the door. Testing this now with changes you've also pushed to mono 👨🏼‍💻

@fbwoolf
Copy link
Contributor Author

fbwoolf commented Jan 23, 2025

Alright let's try getting this out the door. Testing this now with changes you've also pushed to mono 👨🏼‍💻

I ran into an issue last night trying to do a swap. Bc Bitflow sends us ClarityValues from 6.17.0, when we serialize we get an error.

@kyranjamie kyranjamie force-pushed the refactor/upgrade-stacks-js branch from 199c19e to 3a79248 Compare January 23, 2025 12:48
Sign up for free to 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.

2 participants