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

SDK - Tx Storage & Ark SDK func update #320

Open
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

sekulicd
Copy link
Collaborator

@sekulicd sekulicd commented Sep 17, 2024

This adds storage for transactions and vtxos into SDK sqlite storage.
This refactors ark sdk api by adding GetTransactionEventChannel which can be used to listen for new events, it replaces PaymentNotification that was discussed before, this to be closed.
It also updates GetTransactionHistory to read txs from db.
Main idea is to add listenForVtxos function that polls the server every 1(or smtn diff) seconds for new VTXOs,
Func listenToVtxoChan will listen for new vtxos and with help of current state in db will create tx history by using existing
vtxosToTxsCovenant/vtxosToTxsCovenantless funcs.

I pushed changes to the same PR in which i emphasised potential bug in how we handle covenant transaction history, PSB:

Although, this could chain in near future, ATM, async(pending) payments are related to BTC chain(covenantless) 
while regular(NON-PENDING) payments are related to Liquid chain(covenant).

This adds test case, for covenant GetTransactionHistory, that check bellow scenario(bellow u can see 
expectations as well):
1. before onboarding => empty history
2. after sending sats to the boarding address => 1 PENDING boarding tx
3. after claiming the boarded funds => 1 NON-PENDING boarding tx
4. after sending N sats to bob => 1 NON-PENDING sent tx, 1 NON-PENDING boading tx
Test is failing cause current code seems that it only supports BTC chain as it ignores non pending payments, here.
For covenant GetTransactionHistory returns only boarding txs.

@bordalix let me know if u agree that this is bug.

@tiero @louisinger @bordalix @altafan please review.

- Add listenForVtxos function to poll server for new VTXOs
- Implement listenToVtxoChan to process and insert VTXOs and transactions
- Handle both spendable and spent VTXOs, as well as boarding transactions
- Use goroutines and channels for asynchronous processing
- Add error handling and logging for better debugging
@sekulicd sekulicd changed the title Test Covenant GetTransactionHistory SDK - Tx Storage Sep 20, 2024
@tiero
Copy link
Member

tiero commented Sep 20, 2024

is the title correct? Only touches the covenant version? We should mention a new big feature like adding a SQLite storage. Also how that will impact WASM wrapper?

@sekulicd sekulicd changed the title SDK - Tx Storage SDK - Tx Storage & Ark SDK method update Sep 20, 2024
@sekulicd sekulicd changed the title SDK - Tx Storage & Ark SDK method update SDK - Tx Storage & Ark SDK func update Sep 20, 2024
@bordalix
Copy link
Collaborator

@sekulicd we made some changes to covenant's GetTransactionHistory() yesterday.
Please merge master into your branch and check if error persists.

Copy link
Collaborator

@altafan altafan left a comment

Choose a reason for hiding this comment

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

Better to start with KV implementation of the repos (badger) instead of sqlite. We'll make it available once the persistency layer of the SDK client is validated.

explorer explorer.Explorer
client client.ASPClient

vtxosChan chan map[spent]client.Vtxo
Copy link
Collaborator

Choose a reason for hiding this comment

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

can you explain why map[spent]client.Vtxo?

Comment on lines +157 to +175
func (a *arkClient) ListVtxos(
ctx context.Context,
) (spendableVtxos, spentVtxos []client.Vtxo, err error) {
offchainAddrs, _, _, err := a.wallet.GetAddresses(ctx)
if err != nil {
return
}

for _, addr := range offchainAddrs {
spendable, spent, err := a.client.ListVtxos(ctx, addr)
if err != nil {
return nil, nil, err
}
spendableVtxos = append(spendableVtxos, spendable...)
spentVtxos = append(spentVtxos, spent...)
}

return
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

after syncing with master this must be dropped

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.

4 participants