Add support for `from` address in `kaspawallet send` #1964

tmrlvi · 2022-03-03T11:34:29Z

Add an option to choose the from address in a transactions when sending KAS
Do not use change address is change is 0 (transaction with 0 output is not accepted by kaspad anyway)

tmrlvi · 2022-03-03T11:37:11Z

This solves Issue #1957

codecov · 2022-03-06T22:41:49Z

Codecov Report

Merging #1964 (9ba4013) into dev (8d5faee) will decrease coverage by 0.05%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##              dev    #1964      +/-   ##
==========================================
- Coverage   59.51%   59.45%   -0.06%     
==========================================
  Files         675      675              
  Lines       32201    32213      +12     
==========================================
- Hits        19165    19153      -12     
- Misses      10276    10309      +33     
+ Partials     2760     2751       -9

Impacted Files	Coverage Δ
cmd/kaspawallet/config.go	`0.00% <ø> (ø)`
cmd/kaspawallet/create_unsigned_tx.go	`0.00% <0.00%> (ø)`
...allet/daemon/server/create_unsigned_transaction.go	`0.00% <0.00%> (ø)`
cmd/kaspawallet/daemon/server/send.go	`0.00% <0.00%> (ø)`
cmd/kaspawallet/send.go	`0.00% <0.00%> (ø)`
...consensus/database/serialization/acceptancedata.go	`42.85% <0.00%> (-36.74%)`	⬇️
...tures/acceptancedatastore/acceptance_data_store.go	`53.65% <0.00%> (-19.52%)`	⬇️
infrastructure/network/addressmanager/store.go	`77.84% <0.00%> (-1.27%)`	⬇️
infrastructure/network/rpcclient/rpcclient.go	`62.92% <0.00%> (+2.24%)`	⬆️
...rver/grpcserver/protowire/rpc_get_mempool_entry.go	`72.72% <0.00%> (+6.06%)`	⬆️
... and 4 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8d5faee...9ba4013. Read the comment docs.

someone235

In addition to what I wrote in the comments, I think it's better to wait for #1951 to be merged before merging this PR, so the compounded transaction will also be created only from the FromAddresses.

someone235 · 2022-03-09T19:34:41Z

cmd/kaspawallet/daemon/server/create_unsigned_transaction.go

@@ -28,9 +28,17 @@ func (s *server) CreateUnsignedTransaction(_ context.Context, request *pb.Create
 		return nil, err
 	}

+	var fromAddress util.Address = nil


nit: The =nil here is redundant

someone235 · 2022-03-09T19:35:39Z

cmd/kaspawallet/config.go

@@ -54,13 +54,15 @@ type sendConfig struct {
 	Password      string  `long:"password" short:"p" description:"Wallet password"`
 	DaemonAddress string  `long:"daemonaddress" short:"d" description:"Wallet daemon server to connect to (default: localhost:8082)"`
 	ToAddress     string  `long:"to-address" short:"t" description:"The public address to send Kaspa to" required:"true"`
+	FromAddress   string  `long:"from-address" short:"a" description:"Specific public address to send Kaspa from" required:"false"`


If it's not a big deal, I would prefer to directly implement it with multiple addresses (FromAddresses)

someone235 · 2022-03-09T19:39:10Z

cmd/kaspawallet/daemon/server/create_unsigned_transaction.go

@@ -68,7 +80,9 @@ func (s *server) selectUTXOs(spendAmount uint64, feePerInput uint64) (
 	}

 	for _, utxo := range s.utxosSortedByAmount {
-		if !isUTXOSpendable(utxo, dagInfo.VirtualDAAScore, s.params.BlockCoinbaseMaturity) {
+		addr, err := s.walletAddressString(utxo.address)
+		if err != nil || (from != nil && addr != from.String()) ||


Why do you swallow the error?

I thought to ignore invalid UTXOs (which cannot be formatted to address). Maybe a better approach would be to convert from to walletAddress type and explicitly throw an error if failing. I'll look into it

… 0 change

tmrlvi · 2022-04-15T17:03:32Z

I've updated the coded with changes to FromAddresses (plural)
I'm still testing (don't merge yet)

tmrlvi · 2022-04-15T23:12:26Z

Fixed and tested

tmrlvi · 2022-04-15T23:31:55Z

Discussion point: currently, I implement multiple addresses using comma, which make argument parsing a bit cumbersome. There are two option to make it better (that I can think of):

Use go-flags native option for multiple values (repeating flags)
Implement custom parser for go-flags (as described in https://stackoverflow.com/a/28323276/3051715)

Do you think we should change the parsing, or is the way it is written now ok?

someone235 · 2022-04-26T21:42:51Z

cmd/kaspawallet/daemon/server/create_unsigned_transaction.go

@@ -83,7 +98,8 @@ func (s *server) selectUTXOs(spendAmount uint64, feePerInput uint64) (
 	}

 	for _, utxo := range s.utxosSortedByAmount {
-		if !isUTXOSpendable(utxo, dagInfo.VirtualDAAScore, s.params.BlockCoinbaseMaturity) {
+		if (fromAddresses != nil && !slices.Contains(fromAddresses, utxo.address)) ||


Nice! First use of generics in the codebase!

someone235 · 2022-04-26T21:47:24Z

Discussion point: currently, I implement multiple addresses using comma, which make argument parsing a bit cumbersome. There are two option to make it better (that I can think of):

Use go-flags native option for multiple values (repeating flags)

Implement custom parser for go-flags (as described in https://stackoverflow.com/a/28323276/3051715)

Do you think we should change the parsing, or is the way it is written now ok?

I think it's better to stick with the go-flags standard

tmrlvi · 2022-04-26T23:28:25Z

Discussion point: currently, I implement multiple addresses using comma, which make argument parsing a bit cumbersome. There are two option to make it better (that I can think of):

Use go-flags native option for multiple values (repeating flags)

Implement custom parser for go-flags (as described in https://stackoverflow.com/a/28323276/3051715)

Do you think we should change the parsing, or is the way it is written now ok?

I think it's better to stick with the go-flags standard

Ok, so I'm changing --from-addresses to --from-address, and allow for multiple uses. Example:

./kaspawallet create-unsigned-transaction -a <from_addr_1> -a <from_addr_2> -t <to_addr> -v <value>

someone235 requested changes Mar 9, 2022

View reviewed changes

tmrlvi added 3 commits April 15, 2022 18:03

Added option to choose from address in kaspawallet + fixed a bug with…

30a9552

… 0 change

Checking if From is missing

84f2eff

Fixed after merge to kaspad0.12

c6fc723

tmrlvi force-pushed the from_address branch from 607d6ee to c6fc723 Compare April 15, 2022 16:57

tmrlvi added 2 commits April 15, 2022 20:00

Applying changes also to send command

e1a6eed

Better help description

83e6ef3

Fixed bug where we take all utxos except the one we wanted

55f1391

Swallow the parsing error only when we want to filter

762bd24

checking for wallet address directly

a5e2a12

tmrlvi requested a review from someone235 April 17, 2022 09:51

Merge branch 'dev' into from_address

4161957

someone235 reviewed Apr 26, 2022

View reviewed changes

go fmt

1895fbd

tmrlvi added 2 commits April 27, 2022 02:29

Changing to --from-address

4b48a47

Merge branch 'dev' into from_address

9ba4013

someone235 approved these changes Apr 26, 2022

View reviewed changes

someone235 merged commit 540b0d3 into kaspanet:dev Apr 27, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for `from` address in `kaspawallet send` #1964

Add support for `from` address in `kaspawallet send` #1964

tmrlvi commented Mar 3, 2022

tmrlvi commented Mar 3, 2022

codecov bot commented Mar 6, 2022 •

edited

Loading

someone235 left a comment •

edited

Loading

someone235 Mar 9, 2022

someone235 Mar 9, 2022

someone235 Mar 9, 2022

tmrlvi Apr 4, 2022

tmrlvi commented Apr 15, 2022

tmrlvi commented Apr 15, 2022

tmrlvi commented Apr 15, 2022 •

edited

Loading

someone235 Apr 26, 2022

someone235 commented Apr 26, 2022

tmrlvi commented Apr 26, 2022

Add support for from address in kaspawallet send #1964

Add support for from address in kaspawallet send #1964

Conversation

tmrlvi commented Mar 3, 2022

tmrlvi commented Mar 3, 2022

codecov bot commented Mar 6, 2022 • edited Loading

Codecov Report

someone235 left a comment • edited Loading

Choose a reason for hiding this comment

someone235 Mar 9, 2022

Choose a reason for hiding this comment

someone235 Mar 9, 2022

Choose a reason for hiding this comment

someone235 Mar 9, 2022

Choose a reason for hiding this comment

tmrlvi Apr 4, 2022

Choose a reason for hiding this comment

tmrlvi commented Apr 15, 2022

tmrlvi commented Apr 15, 2022

tmrlvi commented Apr 15, 2022 • edited Loading

someone235 Apr 26, 2022

Choose a reason for hiding this comment

someone235 commented Apr 26, 2022

tmrlvi commented Apr 26, 2022

Add support for `from` address in `kaspawallet send` #1964

Add support for `from` address in `kaspawallet send` #1964

codecov bot commented Mar 6, 2022 •

edited

Loading

someone235 left a comment •

edited

Loading

tmrlvi commented Apr 15, 2022 •

edited

Loading