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

send-dfi bot to fix immature UTXOs issue #101

Closed
wants to merge 0 commits into from

Conversation

mrgrauel
Copy link

@mrgrauel mrgrauel commented Oct 15, 2022

It's a workaround bot for #100.

It looks for DFI (and UTXO) which it tries to send to toAddress.

Another possibility use case is moving funds from a maxi to any other wallet, like a reinvest to DFX Staking or another Maxi etc.

Build

npm run build:send-dfi

Setup

Add following paramters to the AWS Parameter Store

  • defichain-maxi/wallet-send/toAddress
  • defichain-maxi/wallet-send/fromAddress

Optional you can add:

  • defichain-maxi/settings-send/threshold (Default: 1)
    • Threshold until it tries to send funds
  • defichain-maxi/settings-send/sendall (Default: false)
    • should I send all DFI and UTXO

@mrgrauel mrgrauel changed the title 🚧 Workaround for immature UTXO Workaround Bot for immature UTXO Oct 15, 2022
@mrgrauel mrgrauel marked this pull request as draft October 15, 2022 22:13
@mrgrauel mrgrauel changed the title Workaround Bot for immature UTXO 🚧 Workaround Bot for immature UTXO Oct 15, 2022
@mrgrauel
Copy link
Author

Changed it to draft, because I have an idea which I'd like to try, but if you want please give me initial feedback.

@mrgrauel
Copy link
Author

Idea

I have multiple Vault Maxis, but I want to push a specific maxi in the next couple of days / weeks / months. So I'd like to push all rewards to a specific address.

I'll rename the bot to send-dfi and also look for DFI token. By default the bot leaves 1 UTXO in the wallet for transaction fees, but an optional parameter can be used to use sendall() which is better for the masternode rewards workaround.

@mrgrauel mrgrauel marked this pull request as ready for review October 16, 2022 18:50
@mrgrauel mrgrauel changed the title 🚧 Workaround Bot for immature UTXO send-dfi Bot to fix immature UTXO issue Oct 16, 2022
@mrgrauel mrgrauel changed the title send-dfi Bot to fix immature UTXO issue send-dfi bot to fix immature UTXOs issue Oct 16, 2022
@@ -60,6 +61,8 @@ export class StoredSettings {
mainCollateralAsset: string = 'DFI'
stableCoinArbBatchSize: number = -1
reinvestThreshold: number | undefined
sendThreshold: number | undefined
sendAll: boolean = false
Copy link
Owner

Choose a reason for hiding this comment

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

probably time to refactor this, so that every program-store has its own stored settings. right now this is becoming a mix of all of them.

Comment on lines 60 to 71
const accountToUtxos: AccountToUtxos = {
from: this.script!,
balances: [{ token: 0, amount: fromBalance }],
mintingOutputsStart: 2,
}

const txn = await account?.withTransactionBuilder()?.account.accountToUtxos(accountToUtxos, this.script!)
if (txn === undefined) {
await telegram.send('ERROR: transactionSegWit is undefined')
console.error('transactionSegWit is undefinedd')
return false
}
Copy link
Owner

Choose a reason for hiding this comment

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

building tx normally happens in common-program. There you can use the already proven ways to build defiTx. then you also don't need to make this.script protected.

Comment on lines 78 to 95
const decodedToAddress = fromAddress(this.toAddress, this.walletSetup.network.name)
if (decodedToAddress === undefined) {
console.error('decodedFromAddress is undefined')
await telegram.send('ERROR: decodedFromAddress is undefined')
return false
}
let txn: TransactionSegWit | undefined
if (this.sendAll) {
txn = await account?.withTransactionBuilder()?.utxo.sendAll(decodedToAddress.script)
} else {
txn = await account?.withTransactionBuilder()?.utxo.send(amountToUse, decodedToAddress.script, this.script!)
}

if (txn === undefined) {
await telegram.send('ERROR: transactionSegWit is undefined')
console.error('transactionSegWit is undefinedd')
return false
}
Copy link
Owner

Choose a reason for hiding this comment

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

again, this would be better in common program. then it can be reused by other programs when they want to send UTXOs

Comment on lines 85 to 89
if (this.sendAll) {
txn = await account?.withTransactionBuilder()?.utxo.sendAll(decodedToAddress.script)
} else {
txn = await account?.withTransactionBuilder()?.utxo.send(amountToUse, decodedToAddress.script, this.script!)
}
Copy link
Owner

Choose a reason for hiding this comment

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

both case are effectivly "send amountToUse" right? why have it seperated here?

return false
}

if (!(await this.sendAndWait(txn, telegram))) {
Copy link
Owner

Choose a reason for hiding this comment

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

if you use it as prevout, you don't need to wait and can add the sendUTXO tx right away. -> only need to wait onces at the end.

@kuegi
Copy link
Owner

kuegi commented Oct 17, 2022

but an optional parameter can be used to use sendall() which is better for the masternode rewards workaround.

@mrgrauel does sendall something different than combining all UTXOs? I think send-all won't work with immature UTXOs either, right?

@mrgrauel mrgrauel marked this pull request as draft October 17, 2022 06:55
@mrgrauel
Copy link
Author

It doesn't work either, it's just an easy way to guarantee that everything is send.

I'll try to use common-programm.ts instead of the transaction builder 👍 I hope to find time some evening this week.

@Krysh90
Copy link
Collaborator

Krysh90 commented Oct 17, 2022

@kuegi using common-program build defi tx works for all OP_DEFI_TXs but sending UTXOs is not an OP_DEFI_TX. Thats why I have said to @mrgrauel that it will be easier to make two transactions with transaction builder instead of refactoring common-program that both raw-txs are working. Esp. if somebody is just starting with jellyfish it is hard to write raw-txs right from the start. Yes this can and should be improved, but i don't know if this PR is the right place to do so

@kuegi
Copy link
Owner

kuegi commented Oct 18, 2022

@kuegi using common-program build defi tx works for all OP_DEFI_TXs but sending UTXOs is not an OP_DEFI_TX. Thats why I have said to @mrgrauel that it will be easier to make two transactions with transaction builder instead of refactoring common-program that both raw-txs are working. Esp. if somebody is just starting with jellyfish it is hard to write raw-txs right from the start. Yes this can and should be improved, but i don't know if this PR is the right place to do so

agreed on the send of utxo, but the account to utxo should move to common. I anyway need those two for the new reinvest feature. will add them in there, then it can be used here too.

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