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

WhalePrevoutProvider should ignore UTXO that are Immature (<100 confirmations) #1819

Open
Tracked by #1818
fuxingloh opened this issue Oct 18, 2022 · 11 comments
Open
Tracked by #1818
Assignees
Labels
area/packages kind/feature New feature request priority/important-soon Will be important soon triage/accepted Triage has been accepted

Comments

@fuxingloh
Copy link
Member

We can throw an error when attempting to spend immature UTXO. We can improve and expand on the basic PrevoutProvider while we're at it.

/triage accepted
/area packages

@github-actions github-actions bot added triage/accepted Triage has been accepted needs/kind Needs kind label area/packages labels Oct 18, 2022
@fuxingloh
Copy link
Member Author

/priority important-soon

@fuxingloh
Copy link
Member Author

/kind feature

@github-actions github-actions bot added kind/feature New feature request and removed needs/kind Needs kind label labels Oct 18, 2022
@fuxingloh
Copy link
Member Author

/assign @thedoublejay for assignment to @WavesHQ since it directly affects Wallet UX. Not urgent or blocking, do it at your own org pace.

@DieHard073055
Copy link
Contributor

Do I need to update these 2 functions to ignore the UTXO's that are immature?
https://github.com/JellyfishSDK/jellyfish/blob/main/packages/whale-api-wallet/src/prevout.ts#L18-L39
If so how would I get the number of confirmations for a specific UTXO?

@DieHard073055
Copy link
Contributor

@DieHard073055
Copy link
Contributor

Also how would I run a test of some sort so I can step through the code to look at what I am working with?

@fuxingloh
Copy link
Member Author

@canonbrother gave me this.

const txn = await this.client.rawtx.getRawTransaction(txid, true)

https://github.com/JellyfishSDK/jellyfish/blob/811b25814de3204b21e84e8ccccc82dda75410ae/packages/jellyfish-api-core/src/category/rawtx.ts#L328

rawtx.ts

Yes, but it's too expensive to call this.

You can get the current block height by using this.client.blockchain.getBlockCount() and probably .filter() and calculate the confirmation manually.

Fortunately,

https://github.com/JellyfishSDK/jellyfish/blob/811b25814de3204b21e84e8ccccc82dda75410ae/packages/whale-api-client/src/api/address.ts#L191

contains block.height so the call won't be expensive.

@DieHard073055
Copy link
Contributor

  1. If I were to write a unit test, how should it look like? Is there any other test I can refer to?

  2. Should the test be at

packages/whale-api-wallet/__tests__

@fuxingloh
Copy link
Member Author

  1. If I were to write a unit test, how should it look like? Is there any other test I can refer to?
  2. Should the test be at
packages/whale-api-wallet/__tests__

You can try looking at existing literature on how PRs are created for whale-api related change.

I assumed it's packages/whale-api-wallet/__tests__; you can give it a go — I will review it afterward if it's awkwardly placed.

@DieHard073055
Copy link
Contributor

Looking at the most relevant test (whale.rpc.client.test.ts) in whale-api-client.

I need to instantiate the following.

  • WhalePrevoutProvider
    • WhaleWalletAccount
      • WalletEllipticPair
      • Network
  • MasterNodeRegTestContainer
  • StubService

I am not sure how I can create the WalletEllipticPair type object.

@DieHard073055
Copy link
Contributor

How should I instantiate the wallet which needs to be passed into the WhaleWalletAccountProvider ?
https://github.com/JellyfishSDK/jellyfish/pull/1941/files#diff-feb1151a3ee35e10dfbcbb91329d4a960ee8e7137d07d5a3cedb0fa6268a5c18R90

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/packages kind/feature New feature request priority/important-soon Will be important soon triage/accepted Triage has been accepted
Projects
None yet
3 participants