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

Simplify nodejs client and wallet #1056

Closed
wants to merge 74 commits into from
Closed

Simplify nodejs client and wallet #1056

wants to merge 74 commits into from

Conversation

DaughterOfMars
Copy link

@DaughterOfMars DaughterOfMars commented Aug 21, 2023 •
edited
Loading

Description of change

This PR cleans up the client and wallet nodejs rust bindings.

Links to any relevant issues

Closes #699

thibault-martinez and others added 30 commits July 24, 2023 08:29
@thibault-martinez
Enable workflows (#376)
161a79e
@thibault-martinez
Remove ReceiptMilestoneOption (#375)
f154741 
* Remove Receipt

* Remove from bindings
@thibault-martinez
Remove treasury related types (#383)
a791416 
* Remove TreasuryTransactionPayload

* Remove TreasuryInput

* Remove TreasuryOutput

* Fmt

* Remove from bindings

* Nit

* Fix test
@thibault-martinez
Remove milestone related types and functions (#402)
bc1bac3 
* Remove milestone related types and functions

* Remove milestone option module

* Remove milestoneMethods.spec.ts

* Remove more receipts

* Fixes

* Remove treasury

* moar

* MOAR

* Fix wasm?

* Fix tests

* Review comments

* Clippy

* More remove

* Remove TailTransactionHashNotUnique
@thibault-martinez
Bump indexer and core routes versions (#641)
6106765 
* Bump indexer routes versions

* Bump core routes versions

* Temporarily disable private tangle tests

* Fix private tangle ignore

* Ignore wasm tests
@thibault-martinez @Thoralf-M
Add slot related types (#642)
2474b9f 
* Add SlotIndex

* Slot module

* Add SlotCommitmentId

* Add SlotCommitment::id

* Add RootsId

* Add SlotCommitment

* Add deref

* Added getters

* Use internal index

* Update sdk/src/types/block/slot/commitment.rs

Co-authored-by: Thoralf-M <[email protected]>

* Update sdk/src/types/block/slot/commitment.rs

Co-authored-by: Thoralf-M <[email protected]>

---------

Co-authored-by: Thoralf-M <[email protected]>
@thibault-martinez
Update InfoResponse (#648)
8f5fb2c 
* Remove WhiteFlagResponse

* Update InfoResponse

* Add IssuerId

* Update ProtocolParameters

* Remove check_existing_db tests

* Fix test

* fix

* Revert triple slash
@thibault-martinez
Add commitment routes (#651)
57a77c2 
* Add Commitments routes

* Fix no_std

* Fix get_slot_commitment_by_index_raw name

* Add GET

* derive_more Display
@thibault-martinez @Thoralf-M
OutputMetadata 2.0 update (#655)
227f887 
* Update OutputMetadata

* Fix test

* Nit

* review comment

* Nits

* Add missing fields

* Fix test

* Update sdk/src/types/block/output/metadata.rs

Co-authored-by: Thoralf-M <[email protected]>

---------

Co-authored-by: Thoralf-M <[email protected]>
@thibault-martinez
Remove metadata from output endpoint (#673)
62c5920 
* Remove metadata from output endpoint

* Fix compilation

* Remove wallet nodejs workflow

* Use get_output_raw

* Move get_output_with_metadata
@Thoralf-M
Remove client block builder (#674)
08f4e72 
* Remove client block builder

* Add imports

* Remove unused import

* Fix example

* Add issue link
@thibault-martinez
Add UTXO changes 2.0 endpoints (#695)
e4e7737 
* Add UTXO changes endpoints

* Remove index refs
@thibault-martinez
RegularTransactionEssence 2.0 changes (#696)
f957da7 
* Bump RegularTransactionEssence kind

* Add creation_time field

* Fix tests

* Actual creation time

* Fix no_std

* Add links to the issue, for some reason
@thibault-martinez
StrongParents, WeakParents and ShallowLikeParents (#697)
15df0ac 
* StrongParents, WeakParents and ShallowLikeParents

* Fix
@thibault-martinez
Less nested ISA (#698)
c66b974
@thibault-martinez @Thoralf-M
Rename Alias to Account (#706)
c01dcc5 
* Rename Alias to Account

* More rename

* More rename

* More rename

* More rename

* More rename

* More rename

* More rename

* More rename

* More rename

* More rename

* More rename

* Last renames

* Disable yarn tests

* trigger

* Trigger

* Rename files

* Fix

* Update sdk/examples/wallet/16_destroy_account_output.rs

Co-authored-by: Thoralf-M <[email protected]>

* More rename

* Update sdk/src/types/block/address/account.rs

Co-authored-by: Thoralf-M <[email protected]>

* Update sdk/src/types/block/output/unlock_condition/immutable_account_address.rs

Co-authored-by: Thoralf-M <[email protected]>

* Review comments

* More rename

* Update sdk/tests/types/address/account.rs

Co-authored-by: Thoralf-M <[email protected]>

* Update sdk/tests/types/address/account.rs

Co-authored-by: Thoralf-M <[email protected]>

* Update sdk/examples/wallet/16_destroy_account_output.rs

Co-authored-by: Thoralf-M <[email protected]>

* Update sdk/src/wallet/account/operations/transaction/high_level/create_account.rs

Co-authored-by: Thoralf-M <[email protected]>

* Add TODO

* Fix create_account_output_command rename

---------

Co-authored-by: Thoralf-M <[email protected]>
@thibault-martinez @Alex6323
Add DelegationOutput (#719)
14ed357 
* Add DelegationOutput

* Nit

* Fix a match

* Add delegation to chain ID

* Fix compilation

* Clippy

* Docs

* Update sdk/src/types/block/output/delegation.rs

Co-authored-by: /alex/ <[email protected]>

* Update sdk/src/types/block/output/delegation.rs

Co-authored-by: /alex/ <[email protected]>

* Remove doc

* Nits

* Fmt and audit

---------

Co-authored-by: /alex/ <[email protected]>
@thibault-martinez
Add RegularTransactionEssenceBuilder::with_creation_time for DTOs (+ …
ead9aa4 
…python fmt) (#896)

* Add RegularTransactionEssenceBuilder::with_creation_time for DTOs

* Python fmt

* Uncomment license

* Remove itertools

* Disable private tangle
@thibault-martinez
DelegationOutput doesn't have immutable features anymore (#921)
c877752
Add mana field to rust outputs (#922)
458e47b 
* Add mana to rust basic output

* Add mana to rust Account and Nft outputs

* clippy

* fix

* forgot packing

* Fix nodejs mqtt example

* Fix tests

* temp remove tests

* defaults

* TODOs
@thibault-martinez
Delete migrations (#927)
0725245 
Co-authored-by: Thibault Martinez <[email protected]>
@thibault-martinez
Add parents and burned_mana to rust block (#925)
0c13a12 
* Add parents to block

* Add burned mana to rust block (#926)

* merge imports

* udep

* Comment out more tests that will change

* oop

---------

Co-authored-by: Thibault Martinez <[email protected]>
@Thoralf-M
Remove old nodejs binding (#929)
ba661ff
@thibault-martinez
Remove PoW and Block::nonce (#930)
ce2fea1 
* Remove pow module

* Remove Block::nonce field

* Fix compilation

* Remove pow tests

* More stuff removed

* Remove more stuff

* Remove more stuff

* Remove more stuff

* Fix compilation

* Clippy

* Remove trailing comma

* Fix tests
Fix std dep (#956)
164549b
@thibault-martinez
Factorize get_outputs variants (#939)
ee5baa4 
* Factorize get_outputs variants

* Clippy

* Dots

* Add s

* Add get_outputs_metadata

* Nit

* fix CI

---------

Co-authored-by: Thibault Martinez <[email protected]>
@thibault-martinez
Add StakingFeature (#797)
1d6b18c 
* StakingFeature

* Fix delegation

* fmt

* camelCase

* Nits

* Fix compilation

* Fix kind

* Fmt

* Add serde string

* one import to rule them all
@thibault-martinez
Add PublicKey type (#705)
8ed032c 
* Add PublicKey type

* Fix compilation

* Add is/as

* Remove serde derive

* InvalidPublicKeyKind

* Nit

* Clippy

* Simpler debug

* Signature uses public key
@thibault-martinez
Temporarily disable ignore tests on coverage (#959)
42ed665
@Thoralf-M
Clean up by privatizing more DTOs (#893)
44f2ab7 
* Remove some dtos

* Remove Input dtos

* access

* tidy up

* more

* amounts

* MORE

* MOOOOORRREEE

* clippy

* Fix feature dtos

* cleanup

* more cleanup

* format python

* license

* Fix nodejs mqtt example

* clippy

* Update sdk/src/types/block/signature/ed25519.rs

Co-authored-by: Thoralf-M <[email protected]>

* udep

* no_std

* Fix copy derive

* kind of annoying clippy

* recreate tests with serde

* more clippy annoyances

* PR suggestions

* really? fmt?

---------

Co-authored-by: Thoralf-M <[email protected]>
@DaughterOfMars
Copy link
Author

Because I did a base change and github is bad

@DaughterOfMars
Copy link
Author

Jest has detected the following 1 open handle potentially keeping Jest from exiting:

I think this is because we are no longer returning a promise, so evidence that nodejs needs changes.

@Thoralf-M @thibault-martinez
Add GET /api/core/v3/rewards/{outputId} route (#1066)
019f1e5 
* Add `GET /api/core/v3/rewards/{outputId}` route

* Update sdk/src/types/api/core/response.rs

Co-authored-by: Thibault Martinez <[email protected]>

* Rename

* Format

* Add note

* Update sdk/src/client/node_api/core/routes.rs

Co-authored-by: Alexandcoats <[email protected]>

* Comments

* Comment

* Update sdk/src/client/node_api/core/routes.rs

---------

Co-authored-by: Thibault Martinez <[email protected]>
Co-authored-by: Alexandcoats <[email protected]>
@kwek20
Copy link
Contributor

kwek20 commented Aug 23, 2023

Jest has detected the following 1 open handle potentially keeping Jest from exiting:

I think this is because we are no longer returning a promise, so evidence that nodejs needs changes.

No i've tried to solve this before using the .finish() on client, but i dont think its actually related to our code

@abdulmth
Adapt fields of RegularTransactionEssence in Node.js (#1075)
fabe69c 
* adapt fields

* format

* review suggestions
@DaughterOfMars DaughterOfMars linked an issue Aug 25, 2023 that may be closed by this pull request
Investigate: Nodejs bindings wrapper type #699
Closed
Alexandcoats and others added 10 commits August 25, 2023 12:48
@Thoralf-M
Update info response and related structs (#1028)
51087bf 
* Update info response and related structs

* display

* fix wasm, no_std and test

* fix one more test

* Implement mapped params, epoch index, helpers

* fix tests, wasm

* copy/paste oopsie

Co-authored-by: Thoralf-M <[email protected]>

* tangle time

* suggestions

* more renames

* More cleanup

* as -> to

* harmony and peace

* derive

* suggestion

* another missed derive

* oops

* Always get v3 params for now. Add flexible epoch index calculation for future proofing

* fix wasm

* add a test and impl suggestions

* Update sdk/src/types/block/slot/epoch.rs

Co-authored-by: Thoralf-M <[email protected]>

* Revert "Update sdk/src/types/block/slot/epoch.rs"

This reverts commit 3668f72.

* add another fail condition, a test, and remove TODOs

* Updates and add example comments

* Updates from TIP

* doc comments

* fix test and comment

* fix other test

---------

Co-authored-by: Thoralf-M <[email protected]>
@Thoralf-M
Add GET /api/core/v3/committee/ route (#1055)
d0a6ee7 
* Update info response and related structs

* display

* fix wasm, no_std and test

* fix one more test

* Implement mapped params, epoch index, helpers

* fix tests, wasm

* Add GET /api/core/v3/committee/ route

* copy/paste oopsie

Co-authored-by: Thoralf-M <[email protected]>

* tangle time

* suggestions

* more renames

* More cleanup

* as -> to

* harmony and peace

* derive

* suggestion

* another missed derive

* oops

* Always get v3 params for now. Add flexible epoch index calculation for future proofing

* fix wasm

* fix merge

* PR suggestions

* revert order

---------

Co-authored-by: Thoralf-M <[email protected]>
@thibault-martinez
Rename AccountAddress to Bip44Address (#1098)
0291459 
* Rename AccountAddress to Bip44Address

* Remove used field

* Remove unused type

* Nit

* Nit
fix timestamp <-> slot calculations and add tests (#1093)
404523c
@thibault-martinez
Feat/node: Replace unixTime with slotIndex in Timelock/Expiration (#1092
144959d 
)

* slot index

* case

* tests fix

* format

* readonly

* review

* format

* updated comment

* Nit

---------

Co-authored-by: Thibault Martinez <[email protected]>
@thibault-martinez
Fix protocol parameters integer types (#1094)
766f040 
* Fix protocol parameters integer types

* Fix block ID test

* Update sdk/src/types/block/protocol.rs

Co-authored-by: Alexandcoats <[email protected]>

* Add doc

* Update sdk/src/types/block/protocol.rs

* Update sdk/src/types/block/protocol.rs

Co-authored-by: Alexandcoats <[email protected]>

* Update sdk/src/types/block/protocol.rs

Co-authored-by: Alexandcoats <[email protected]>

* Update sdk/src/types/block/protocol.rs

Co-authored-by: Alexandcoats <[email protected]>

* Nits

* Apply suggestions from code review

Co-authored-by: Alexandcoats <[email protected]>

* sad format noises

---------

Co-authored-by: Alexandcoats <[email protected]>
@thibault-martinez
Rename Alias to Account (#1077)
fdd31ca 
* Rename Alias to Account

* More rename

* Rename files

* Last renames

* Format

* Nits

* Disambiguate AccountAddress

* AccountIdentifier

* Fix wasm test
@Thoralf-M @thibault-martinez
Storage migration (#1057)
ce77da1 
* Brainstorm

* Basic idea

* Add encrypted db

* Seems to work

* Define types in migration, refactor

* where in stronghold is the data?

* Make stronghold snapshot migration work

* Add migration to bindings, add get_chrysalis_data()

* Save chrysalis data from snapshot in db

* Address some TODOs

* Address todos

* Add chrysalis-backup-work-factor-0.stronghold

* Fix wasm

* Don't error on ClientDataNotPresent

* Clippy

* Address review comments

* Remove extra function

* Update bindings/nodejs/CHANGELOG.md

Co-authored-by: Thibault Martinez <[email protected]>

* Remove `from`

* Clippy

* Changelog

* Update sdk/src/wallet/storage/constants.rs

* Move function to storage

* Address review comments

* Address review comments

* Review suggestions

* Address review comments

* Add db_encryption_key to Rust, export migrateDbChrysalisToStardust and add nodejs example

* Single import, lint

* Address review comments

---------

Co-authored-by: Thibault Martinez <[email protected]>
Co-authored-by: Thibault Martinez <[email protected]>
@thibault-martinez
Fix readmes (#1110)
711f85c
@thibault-martinez @Thoralf-M
Add PrivateKeySecretManager (#937)
a8b5390 
* Add PrivateKeySecretManager

* try_from_b58/try_from_hex

* zeroize bytes

* Nits

* Optional bs58

* Changelog

* Secret manager tests folder

* Add tests and fix

* Changelog

* Nit

* Nit

* Update sdk/tests/client/secret_manager/mod.rs

Co-authored-by: Thoralf-M <[email protected]>

* Replace panic with errors

* Remove unwrap

* Under private_key_secret_manager feature

---------

Co-authored-by: Thoralf-M <[email protected]>
@Thoralf-M
Copy link
Member

Tests need to be updated because the error message changed

DaughterOfMars
DaughterOfMars commented Aug 31, 2023
View reviewed changes
bindings/wasm/src/wallet.rs Outdated
@@ -38,38 +57,34 @@ pub fn create_wallet(options: String) -> Result<WalletMethodHandler, JsValue> {
.map_err(|e| e.to_string())?;

Ok(WalletMethodHandler {
wallet: Arc::new(Mutex::new(Some(wallet_method_handler))),
wallet: Arc::new(RwLock::new(Some(wallet_method_handler))),
Copy link
Author

Choose a reason for hiding this comment

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

Why use the std version?

Copy link
Contributor

Choose a reason for hiding this comment

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

we also do in nodejs. but the std one is not async which is the main reason

Copy link
Author

Choose a reason for hiding this comment

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

Yeah but we are in async fns...

Copy link
Contributor

Choose a reason for hiding this comment

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

.client() isnt, just like in rust

Copy link
Author

Choose a reason for hiding this comment

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

pub async fn get_client(method_handler: &WalletMethodHandler) -> Result<ClientMethodHandler, JsValue> {

@Thoralf-M Thoralf-M mentioned this pull request Sep 1, 2023
Closed
@kwek20 kwek20 removed a link to an issue Sep 11, 2023
Investigate: Nodejs bindings wrapper type #699
Closed
@thibault-martinez thibault-martinez deleted the chore/bindings-cleanup branch September 13, 2023 13:37
@kwek20 kwek20 mentioned this pull request Oct 31, 2023
Merged
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Alex6323 Alex6323 Alex6323 left review comments

@thibault-martinez thibault-martinez thibault-martinez left review comments

@kwek20 kwek20 kwek20 left review comments

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

Successfully merging this pull request may close these issues.
7 participants
@DaughterOfMars @thibault-martinez @Thoralf-M @kwek20 @Alex6323 @abdulmth @qrayven