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

Fix spend logic for unlock deposit amount #86

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

knikos
Copy link
Member

@knikos knikos commented Dec 15, 2023

This PR changes the signature of api buildUnlockDepositTx by converting fromAddresses to commonly used FromType and adds the argument amountToUnlock in the builder function to consider it when constructing in/output utxos by calling the node spend logic.

Copy link

⚠️ Found errors/fatal log records. Please review them(job:e2e, step:"Check produced logs") and resolve this comment.

3 errors
0 fatal

@VjeraTurk VjeraTurk self-requested a review August 26, 2024 08:57
@VjeraTurk VjeraTurk self-assigned this Aug 26, 2024
@@ -1443,7 +1445,7 @@ export class Builder {
changeThreshold
)

aad.addAssetAmount(feeAssetID, zero, fee)
aad.addAssetAmount(feeAssetID, amountToUnlock, fee)

const minSpendableErr: Error = await this.spender.getMinimumSpendable(
Copy link

@VjeraTurk VjeraTurk Feb 11, 2025

Choose a reason for hiding this comment

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

  1. call spend for burning the already unlocked UTXOs
  2. call a call to a different function (undeposit ) to unlock the locked UTXOs

* @param changeThreshold Optional. The number of signatures required to spend the funds in the resultant change UTXO
*
* @returns An unsigned transaction created from the passed in parameters.
*/
buildUnlockDepositTx = async (
utxoset: UTXOSet,
fromAddresses: string[],
fromAddresses: FromType,
Copy link

@VjeraTurk VjeraTurk Feb 11, 2025

Choose a reason for hiding this comment

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

Instead of fromAdresses we need depositTxIDs (better than depositOfferId)
We skip the amount for now

@VjeraTurk
Copy link

VjeraTurk commented Feb 11, 2025

buildUnlockDepositTx is not called by caminojs (it would be called by valid) when there's no example of usage.

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