Skip to content

Streamline wallet#55

Merged
SwenSchaeferjohann merged 10 commits intomainfrom
abdul-streamline-wallet
Mar 24, 2023
Merged

Streamline wallet#55
SwenSchaeferjohann merged 10 commits intomainfrom
abdul-streamline-wallet

Conversation

@sleepyqadir
Copy link
Contributor

this pr currently in draft

Streamline Wallet
- just checks with the provider
- the interface is consistent for both the browser wallet and node wallet
- changes to the transaction class etc for a unified interface across browser/native env
- support of spl approve

@SwenSchaeferjohann SwenSchaeferjohann marked this pull request as draft March 23, 2023 13:56
@sleepyqadir sleepyqadir force-pushed the abdul-streamline-wallet branch 2 times, most recently from 398ad3e to 8cc85a6 Compare March 24, 2023 06:47
Abdul Qadir Shaikh and others added 2 commits March 24, 2023 11:49
…ters (#38)

* implemented custom errors in transaction class not tested

* added getAssetPubkeys tests and fixed the fn

* added no u64 errors

* completed deposit tests for transaction parameters

* added functional TransactionParameters tests

* transaction parameters tests work clean functional + errors

* removed transaction class errors after refactor

* removed prints

* fixed verifiers for tx params refactor

* functional tests except user class run

* user class functional tests run

* adapted circuit tests

* verifier two functional test runs

* verifier two tests runs

* adapted remaining system programs tests

* fixed tests after rebase

add custom errors transaction, utxo, verifier classes (#41)

* added custom errors transaction, utxo, verifier classes

* relaxed version requirement on solana web3.js

logs for the testing

changes: added the update merkle tree call

commitment changed in the leaves account data

added the update merkle tree call

Co-authored-by: Abdul Qadir Shaikh <sleepyqadir@Abduls-MacBook-Air.local>

deleted groth16-solana and replaced by crates.io import

light-verifier-sdk: Provide program keys in bs58 format

Use our light-macros crate to provide program IDs to light-verifier-sdk
in bs58 format (which gets converted to bytes during compilation time).

Thanks to that, we also don't have to use the deprecated `Pubkey::new()`
associated method.

add custom errors account (#44)

* added account custom errors

* removed sdk build errors

fixed transaction config ID (#46)

relayer commander for tests

fixed: socket i/o timeout issue

command added

Add Github Actions CI jobs (#32)

* Add Github Actions CI jobs

* Fix cargo fmt issues

* tests: Provide output UTXOs as transaction parameters

ata shield unshield pass in upd (WIP) (#45)

* ata shield unshield pass in upd

enabled: unshield, transfer

user.getBalance:permissioned balances even if 0

added balance checks and todos to unshield user test

unshield -> getTxparams refactor + getRelayer call

transfer -> getTxParams &tested

* tests run: disabled leafindex check in getUnspentUtxos tho

* tests all running

* remove console logs

* removed index checker in getUnspentUtxos

* clean function params getUnspentUtxos

sub module fetched

rm light-macros

cleans the logs

removed the unused relayer script

removed extra consols

useWallet changes

resolved failing test

updated the wait time and verifierZero call

reverted the script change

added script for running the browser wallet tests

fix loopless of browser command

switch to docker solana validator

changes: script changes

changes: added build and install script for relayer

fixed typescript issues in relayer

remove  for unbound issue

cleanups

added relayer custom errors (#49)

* added relayer custom errors

* increased relayer fee in mock-verifier tests

rebase from main

re added the removed package-lock.json

resolve long changes

Jorrit refactor createOutUtxos (#50)

* renamed amount, extraSolAmount, account

* renamed Action.DEPOSIT, WITHDRAWAL, to SHIELD, UNSHIELD

* added error tests

* fixed errors

* removed commented code

* renamed test

* fixed user tests

* added comment

* made errors custom

* improved shielded recipients check for transfer

* renamed tests

Jorrit add custom errors provider (#47)

* added custom errors provider and tests

* fixed rebase conflicts and added a check in createUtxos

* unskipped a test

action enum

checking with finalized

user and provider class refactored

transaction and verifier refactored

spl support and check changed

provider changes and ts errors fixed

changes: errors handles and conditions

remove seperate static calls

checks changes

reset the test scripts

hot fixes in provider for failing test-caes

fixes: user tests cases

light-sdk test cases fixups

verifier tests fixups

nodeWallet sign message changes

long changes reverted

Jorrit refactor select in utxos (#54)

* selectInUtxos functional tests with 2 inputs + custom errors + some error tests

* added failed utxo combination and 3 in utxo tests

* tests pass with refactored selectInUtxos and createUtxos

* removed commented code

* fixed sort

Refactored user and add custom errors (#56)

* refactored user class, and added custom errors

* resolved rebase errors

reverted commits
@sleepyqadir sleepyqadir force-pushed the abdul-streamline-wallet branch from 8cc85a6 to 0af02e8 Compare March 24, 2023 06:58
@sleepyqadir sleepyqadir marked this pull request as ready for review March 24, 2023 12:21
import { sign } from "tweetnacl";

// Mock Solana web3 library
class Provider {
Copy link
Contributor

Choose a reason for hiding this comment

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

this could be quite useful to build more functions such as request utxos, request balance, etc. we will want the wallets to support.
Maybe we should move some stuff from the user, to separate what will be in a wallet and what will be accessible in the application.

cc @SwenSchaeferjohann

Copy link
Contributor

Choose a reason for hiding this comment

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

@sleepyqadir we already have a provider class, what do you think about calling it Wallet?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah Wallet makes more sense, changed it to the Wallet

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Regarding more functions, yes we can add them for sure

transaction,
[this._keypair, ...signers],
{
commitment: "confirmed",
Copy link
Contributor

Choose a reason for hiding this comment

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

We should make this flexible with default "confirmed", we can add it in the constructor

constructor(...,commitment?: string = "confirmed)

this.commitment = commitment;

Copy link
Contributor

@SwenSchaeferjohann SwenSchaeferjohann left a comment

Choose a reason for hiding this comment

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

lgtm! besides some optional/informational comments and a few e..g renamings that i consider useful to change


let balanceUserToken = null;
let userSplAccount = null;
let balanceUserToken: null | any = null;
Copy link
Contributor

Choose a reason for hiding this comment

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

please make the type explicit

signTransaction: (transaction: any) => Promise<any>;
sendAndConfirmTransaction: (transaction: any) => Promise<any>;
publicKey: PublicKey;
node_wallet?: boolean;
Copy link
Contributor

Choose a reason for hiding this comment

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

please rename to isNodeWallet to adhere to naming conventions for bools in es6

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!

Copy link
Contributor

Choose a reason for hiding this comment

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

has this test file been formatted?

let lightProvider = await Provider.native(ADMIN_AUTH_KEYPAIR);


let lightProvider = await Provider.initialize(ADMIN_AUTH_KEYPAIR);
Copy link
Contributor

Choose a reason for hiding this comment

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

.init would be BE/AE-agnostic ..

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Didn't get it

Copy link
Contributor

@ananas-block ananas-block Mar 24, 2023

Choose a reason for hiding this comment

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

in british english it would be initialise, vs AE american initialize, thus swen is suggesting to just do init instead.
I am ok with that

#!/bin/bash -e
../../solana/validator/solana-test-validator --reset --limit-ledger-size 500000000 --faucet-port 9002 --rpc-port 8899 --bpf-program J1RRetZ4ujphU75LP8RadjXMf3sA12yC2R44CF7PmU7i ../light-system-programs/target/deploy/verifier_program_zero.so --bpf-program JA5cjkRJ1euVi9xLWsCJVzsRzEkT8vcC4rqw9sVAo5d6 ../light-system-programs/target/deploy/merkle_tree_program.so --bpf-program 3KS2k14CmtnuVv2fvYcvdrNgC94Y11WETBpMUGgXyWZL ../light-system-programs/target/deploy/verifier_program_one.so --bpf-program noopb9bkMVfRPU8AsbpTUg8AQkHtKwMYZiFUjNRtMmV ../../solana/web3.js/test/fixtures/noop-program/solana_sbf_rust_noop.so --bpf-program Fg6PaFpoGXkYsidMpWTK6W2BeZ7FEfcYkg476zPFsLnS ../light-system-programs/target/deploy/verifier_program.so --quiet &
sleep 7
../../solana/validator/solana-test-validator --reset --limit-ledger-size 500000000 --faucet-port 9002 --rpc-port 8899 --bpf-program J1RRetZ4ujphU75LP8RadjXMf3sA12yC2R44CF7PmU7i ../light-system-programs/target/deploy/verifier_program_zero.so --bpf-program JA5cjkRJ1euVi9xLWsCJVzsRzEkT8vcC4rqw9sVAo5d6 ../light-system-programs/target/deploy/merkle_tree_program.so --bpf-program 3KS2k14CmtnuVv2fvYcvdrNgC94Y11WETBpMUGgXyWZL ../light-system-programs/target/deploy/verifier_program_one.so --bpf-program noopb9bkMVfRPU8AsbpTUg8AQkHtKwMYZiFUjNRtMmV ../../solana/web3.js/test/fixtures/noop-program/solana_sbf_rust_noop.so --bpf-program Fg6PaFpoGXkYsidMpWTK6W2BeZ7FEfcYkg476zPFsLnS ../light-system-programs/target/deploy/verifier_program_storage.so --quiet &
Copy link
Contributor

Choose a reason for hiding this comment

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

curious why did this change ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

verifier_program build as verifier_program_storage

Copy link
Contributor

Choose a reason for hiding this comment

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

but this should already be the case before your commits weird

"moduleResolution": "node",
"rootDirs": ["src", "generated"],
"baseUrl": "src",
"skipLibCheck": true,
Copy link
Contributor

Choose a reason for hiding this comment

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

what does this do/ why do we want to skip it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was trying to resolve the conflicts inside the node_modules, forgot to remove it

// change to bigint
publicAmountSpl.toNumber(),
[this.provider.nodeWallet!],
const transaction = new createTransaction().add(
Copy link
Contributor

Choose a reason for hiding this comment

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

same with createTransaction vs solanaTransaction here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Copy link
Contributor

Choose a reason for hiding this comment

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

ty

assert.equal(lightProviderMock.browserWallet, undefined);
assert.equal(lightProviderMock.nodeWallet?.publicKey.toBase58(), mockKeypair.publicKey.toBase58());
const lightProviderMock = await LightProvider.initialize(mockKeypair);
assert.equal(lightProviderMock.wallet.node_wallet, true);
Copy link
Contributor

Choose a reason for hiding this comment

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

is this correct?
why is node_wallet === true?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah,

lastly we check that the lightProviderMock is initialized with the native and contains the node wallet, but now we have a similar kind of wallet except node wallet contains the node_wallet true

Copy link
Contributor

@ananas-block ananas-block Mar 24, 2023

Choose a reason for hiding this comment

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

then isNodeWallet would be better naming right?
if this variable is always a boolean pls rename lmk in case I misunderstood

Copy link
Contributor

Choose a reason for hiding this comment

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

ah saw this is fixed already

yarn.lock Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

why does this exist?

}

// Mock useWallet hook
export const useWallet = (
Copy link
Contributor

Choose a reason for hiding this comment

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

can we make this a static function of the above class (now provider)?

Copy link
Contributor Author

@sleepyqadir sleepyqadir Mar 24, 2023

Choose a reason for hiding this comment

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

Yes, I guess, should I do it this way,?

signMessage: provider.signMessage,
signTransaction: provider.signTransaction,
signAllTransactions: provider.signAllTransactions,
node_wallet,
Copy link
Contributor

Choose a reason for hiding this comment

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

why don't you return the provider object?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

because, it is how we get the methods from the useWallet in the browser, separately

Copy link
Contributor

@ananas-block ananas-block left a comment

Choose a reason for hiding this comment

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

once test ci errors are resolved and the change initialize to init as Swen suggested,
looks good to me

@SwenSchaeferjohann SwenSchaeferjohann merged commit f7d0181 into main Mar 24, 2023
@SwenSchaeferjohann SwenSchaeferjohann deleted the abdul-streamline-wallet branch March 24, 2023 20:15
vadorovsky pushed a commit to vadorovsky/light-protocol that referenced this pull request May 1, 2024
* added mockUseWallet

* Jorrit add custom errors for transaction class and transaction parameters (Lightprotocol#38)

* implemented custom errors in transaction class not tested

* added getAssetPubkeys tests and fixed the fn

* added no u64 errors

* completed deposit tests for transaction parameters

* added functional TransactionParameters tests

* transaction parameters tests work clean functional + errors

* removed transaction class errors after refactor

* removed prints

* fixed verifiers for tx params refactor

* functional tests except user class run

* user class functional tests run

* adapted circuit tests

* verifier two functional test runs

* verifier two tests runs

* adapted remaining system programs tests

* fixed tests after rebase

add custom errors transaction, utxo, verifier classes (Lightprotocol#41)

* added custom errors transaction, utxo, verifier classes

* relaxed version requirement on solana web3.js

logs for the testing

changes: added the update merkle tree call

commitment changed in the leaves account data

added the update merkle tree call

Co-authored-by: Abdul Qadir Shaikh <sleepyqadir@Abduls-MacBook-Air.local>

deleted groth16-solana and replaced by crates.io import

light-verifier-sdk: Provide program keys in bs58 format

Use our light-macros crate to provide program IDs to light-verifier-sdk
in bs58 format (which gets converted to bytes during compilation time).

Thanks to that, we also don't have to use the deprecated `Pubkey::new()`
associated method.

add custom errors account (Lightprotocol#44)

* added account custom errors

* removed sdk build errors

fixed transaction config ID (Lightprotocol#46)

relayer commander for tests

fixed: socket i/o timeout issue

command added

Add Github Actions CI jobs (Lightprotocol#32)

* Add Github Actions CI jobs

* Fix cargo fmt issues

* tests: Provide output UTXOs as transaction parameters

ata shield unshield pass in upd (WIP) (Lightprotocol#45)

* ata shield unshield pass in upd

enabled: unshield, transfer

user.getBalance:permissioned balances even if 0

added balance checks and todos to unshield user test

unshield -> getTxparams refactor + getRelayer call

transfer -> getTxParams &tested

* tests run: disabled leafindex check in getUnspentUtxos tho

* tests all running

* remove console logs

* removed index checker in getUnspentUtxos

* clean function params getUnspentUtxos

sub module fetched

rm light-macros

cleans the logs

removed the unused relayer script

removed extra consols

useWallet changes

resolved failing test

updated the wait time and verifierZero call

reverted the script change

added script for running the browser wallet tests

fix loopless of browser command

switch to docker solana validator

changes: script changes

changes: added build and install script for relayer

fixed typescript issues in relayer

remove  for unbound issue

cleanups

added relayer custom errors (Lightprotocol#49)

* added relayer custom errors

* increased relayer fee in mock-verifier tests

rebase from main

re added the removed package-lock.json

resolve long changes

Jorrit refactor createOutUtxos (Lightprotocol#50)

* renamed amount, extraSolAmount, account

* renamed Action.DEPOSIT, WITHDRAWAL, to SHIELD, UNSHIELD

* added error tests

* fixed errors

* removed commented code

* renamed test

* fixed user tests

* added comment

* made errors custom

* improved shielded recipients check for transfer

* renamed tests

Jorrit add custom errors provider (Lightprotocol#47)

* added custom errors provider and tests

* fixed rebase conflicts and added a check in createUtxos

* unskipped a test

action enum

checking with finalized

user and provider class refactored

transaction and verifier refactored

spl support and check changed

provider changes and ts errors fixed

changes: errors handles and conditions

remove seperate static calls

checks changes

reset the test scripts

hot fixes in provider for failing test-caes

fixes: user tests cases

light-sdk test cases fixups

verifier tests fixups

nodeWallet sign message changes

long changes reverted

Jorrit refactor select in utxos (Lightprotocol#54)

* selectInUtxos functional tests with 2 inputs + custom errors + some error tests

* added failed utxo combination and 3 in utxo tests

* tests pass with refactored selectInUtxos and createUtxos

* removed commented code

* fixed sort

Refactored user and add custom errors (Lightprotocol#56)

* refactored user class, and added custom errors

* resolved rebase errors

reverted commits

* minor changes

* Revert "minor changes"

This reverts commit ff46ed4.

* minor changes

* added promise await in transaction

* naming conventions

* Revert "naming conventions"

This reverts commit 706b9b8.

* naming conventions

* method name changed

---------

Co-authored-by: Abdul Qadir Shaikh <sleepyqadir@Abduls-MacBook-Air.local>
Co-authored-by: ananas-block <58553958+ananas-block@users.noreply.github.com>
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.

3 participants