-
Notifications
You must be signed in to change notification settings - Fork 232
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
Add support for auto-compound in kaspawallet send
#1951
Changes from 4 commits
bcdf72b
8444a9a
bccca21
504309a
3a91e21
d03d918
2be2632
6218cdb
c7d0267
4abd3f1
78b19d9
529a027
612726e
15369e2
135c08b
7239bc8
dbec7e7
ee60333
3fbf8bc
f21e1be
e90a374
84f3ecd
761d461
bbb7323
75f7190
cb0d618
7a09ed2
608bf69
f39bac4
85accca
e4c453c
c6523bb
e3bac53
c60c339
8cc6187
1cdc3ca
ecf6b57
73bfb61
517063e
3ffacb0
92d9a73
88857c8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,6 +2,7 @@ package server | |
|
||
import ( | ||
"github.com/kaspanet/go-secp256k1" | ||
"github.com/pkg/errors" | ||
|
||
"github.com/kaspanet/kaspad/cmd/kaspawallet/libkaspawallet" | ||
"github.com/kaspanet/kaspad/cmd/kaspawallet/libkaspawallet/serialization" | ||
|
@@ -13,23 +14,19 @@ import ( | |
"github.com/kaspanet/kaspad/util" | ||
) | ||
|
||
func (s *server) maybeAutoCompoundTransaction(transactionBytes []byte) ([][]byte, error) { | ||
func (s *server) maybeAutoCompoundTransaction(transactionBytes []byte, toAddress util.Address, | ||
changeAddress util.Address, changeWalletAddress *walletAddress) ([][]byte, error) { | ||
transaction, err := serialization.DeserializePartiallySignedTransaction(transactionBytes) | ||
if err != nil { | ||
return nil, err | ||
} | ||
|
||
splitAddress, splitWalletAddress, err := s.changeAddress() | ||
if err != nil { | ||
return nil, err | ||
} | ||
|
||
splitTransactions, err := s.maybeSplitTransaction(transaction, splitAddress) | ||
splitTransactions, err := s.maybeSplitTransaction(transaction, changeAddress) | ||
if err != nil { | ||
return nil, err | ||
} | ||
if len(splitTransactions) > 1 { | ||
mergeTransaction, err := s.mergeTransaction(splitTransactions, transaction, splitWalletAddress) | ||
mergeTransaction, err := s.mergeTransaction(splitTransactions, transaction, toAddress, changeAddress, changeWalletAddress) | ||
if err != nil { | ||
return nil, err | ||
} | ||
|
@@ -46,17 +43,17 @@ func (s *server) maybeAutoCompoundTransaction(transactionBytes []byte) ([][]byte | |
return splitTransactionsBytes, nil | ||
} | ||
|
||
func (s *server) mergeTransaction(splitTransactions []*serialization.PartiallySignedTransaction, | ||
originalTransaction *serialization.PartiallySignedTransaction, splitWalletAddress *walletAddress) ( | ||
*serialization.PartiallySignedTransaction, error) { | ||
|
||
targetAddress, err := util.NewAddressScriptHash(originalTransaction.Tx.Outputs[0].ScriptPublicKey.Script, s.params.Prefix) | ||
if err != nil { | ||
return nil, err | ||
} | ||
changeAddress, err := util.NewAddressScriptHash(originalTransaction.Tx.Outputs[1].ScriptPublicKey.Script, s.params.Prefix) | ||
if err != nil { | ||
return nil, err | ||
func (s *server) mergeTransaction( | ||
splitTransactions []*serialization.PartiallySignedTransaction, | ||
originalTransaction *serialization.PartiallySignedTransaction, | ||
toAddress util.Address, | ||
changeAddress util.Address, | ||
changeWalletAddress *walletAddress, | ||
) (*serialization.PartiallySignedTransaction, error) { | ||
numOutputs := len(originalTransaction.Tx.Outputs) | ||
if numOutputs > 2 || numOutputs == 0 { | ||
return nil, errors.Errorf("original transaction has %d outputs, while 1 or 2 are expected", | ||
someone235 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
len(originalTransaction.Tx.Outputs)) | ||
} | ||
|
||
totalValue := uint64(0) | ||
|
@@ -70,29 +67,34 @@ func (s *server) mergeTransaction(splitTransactions []*serialization.PartiallySi | |
Index: 0, | ||
}, | ||
UTXOEntry: utxo.NewUTXOEntry(output.Value, output.ScriptPublicKey, false, constants.UnacceptedDAAScore), | ||
DerivationPath: s.walletAddressPath(splitWalletAddress), | ||
DerivationPath: s.walletAddressPath(changeWalletAddress), | ||
} | ||
totalValue += output.Value | ||
totalValue -= feePerInput | ||
} | ||
|
||
var payments []*libkaspawallet.Payment | ||
if totalValue >= sentValue { | ||
payments = []*libkaspawallet.Payment{{ | ||
Address: targetAddress, | ||
Amount: sentValue, | ||
}, { | ||
if totalValue < sentValue { | ||
// sometimes the fees from compound transactions make the total output higher than what's available from selected | ||
// utxos, in such cases - find one more UTXO and use it. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please update this comment |
||
oneMoreUTXO, err := s.oneMoreUTXOForMergeTransaction(utxos, sentValue-totalValue) | ||
someone235 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if err != nil { | ||
return nil, err | ||
} | ||
utxos = append(utxos, oneMoreUTXO) | ||
totalValue += oneMoreUTXO.UTXOEntry.Amount() | ||
} | ||
|
||
payments := []*libkaspawallet.Payment{{ | ||
Address: toAddress, | ||
Amount: sentValue, | ||
}} | ||
if totalValue > sentValue { | ||
payments = append(payments, &libkaspawallet.Payment{ | ||
Address: changeAddress, | ||
Amount: totalValue - sentValue, | ||
}} | ||
} else { | ||
// sometimes the fees from compound transactions make the total output higher than what's available from selected | ||
// utxos, in such cases, the remaining fee will be deduced from the resulting amount | ||
payments = []*libkaspawallet.Payment{{ | ||
Address: targetAddress, | ||
Amount: totalValue, | ||
}} | ||
}) | ||
} | ||
|
||
mergeTransactionBytes, err := libkaspawallet.CreateUnsignedTransaction(s.keysFile.ExtendedPublicKeys, | ||
s.keysFile.MinimumSignatures, payments, utxos) | ||
if err != nil { | ||
|
@@ -103,7 +105,7 @@ func (s *server) mergeTransaction(splitTransactions []*serialization.PartiallySi | |
} | ||
|
||
func (s *server) maybeSplitTransaction(transaction *serialization.PartiallySignedTransaction, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This approach of splitting a transaction doesn't sit well with me. I couldn't figure out a concrete example, but I'm certain there could be edge-cases where this could produce split transactions over the mass limit. What more, we never even check that the transactions created by this method are valid Instead, I propose a far less efficient, but safe approach:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I feel ya, and also @someone235 has defined said edge case, but I have a feeling what you suggest is an overkill. It also will probably be quite annoying, clocking at O(n^2) with a possibly high n, and high constants (with cloning of the tx, etc.), in an operation that is supposed to be instantaneous. I think the way I did it now is much more foolproof. |
||
splitAddress util.Address) ([]*serialization.PartiallySignedTransaction, error) { | ||
changeAddress util.Address) ([]*serialization.PartiallySignedTransaction, error) { | ||
|
||
transactionMass, err := s.estimateMassAfterSignatures(transaction) | ||
if err != nil { | ||
|
@@ -119,13 +121,16 @@ func (s *server) maybeSplitTransaction(transaction *serialization.PartiallySigne | |
splitCount++ | ||
} | ||
inputCountPerSplit := len(transaction.Tx.Inputs) / splitCount | ||
if len(transaction.Tx.Inputs)%splitCount > 0 { | ||
splitCount++ | ||
} | ||
|
||
splitTransactions := make([]*serialization.PartiallySignedTransaction, splitCount) | ||
for i := 0; i < splitCount; i++ { | ||
startIndex := i * inputCountPerSplit | ||
endIndex := startIndex + inputCountPerSplit | ||
someone235 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
var err error | ||
splitTransactions[i], err = s.createSplitTransaction(transaction, splitAddress, startIndex, endIndex) | ||
splitTransactions[i], err = s.createSplitTransaction(transaction, changeAddress, startIndex, endIndex) | ||
if err != nil { | ||
return nil, err | ||
} | ||
|
@@ -137,18 +142,18 @@ func (s *server) maybeSplitTransaction(transaction *serialization.PartiallySigne | |
func (s *server) createSplitTransaction(transaction *serialization.PartiallySignedTransaction, | ||
changeAddress util.Address, startIndex int, endIndex int) (*serialization.PartiallySignedTransaction, error) { | ||
|
||
selectedUTXOs := make([]*libkaspawallet.UTXO, endIndex-startIndex) | ||
selectedUTXOs := make([]*libkaspawallet.UTXO, 0, endIndex-startIndex) | ||
totalSompi := uint64(0) | ||
|
||
for i := startIndex; i < endIndex; i++ { | ||
for i := startIndex; i < endIndex && i < len(transaction.PartiallySignedInputs); i++ { | ||
partiallySignedInput := transaction.PartiallySignedInputs[i] | ||
selectedUTXOs[i-startIndex] = &libkaspawallet.UTXO{ | ||
selectedUTXOs = append(selectedUTXOs, &libkaspawallet.UTXO{ | ||
Outpoint: &transaction.Tx.Inputs[i].PreviousOutpoint, | ||
UTXOEntry: utxo.NewUTXOEntry( | ||
partiallySignedInput.PrevOutput.Value, partiallySignedInput.PrevOutput.ScriptPublicKey, | ||
false, constants.UnacceptedDAAScore), | ||
DerivationPath: partiallySignedInput.DerivationPath, | ||
} | ||
}) | ||
|
||
totalSompi += selectedUTXOs[i-startIndex].UTXOEntry.Amount() | ||
totalSompi -= feePerInput | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please move this method to split_transaction.go, close to where it's used