tx.author addDestination to verify address#117
Closed
ReevesAk wants to merge 10 commits intoplanetdecred:masterfrom
Closed
tx.author addDestination to verify address#117ReevesAk wants to merge 10 commits intoplanetdecred:masterfrom
ReevesAk wants to merge 10 commits intoplanetdecred:masterfrom
Conversation
oluwandabira
suggested changes
Mar 18, 2020
txauthor.go
Outdated
| func (tx *TxAuthor) AddSendDestination(address string, atomAmount int64, sendMax bool, wallet *Wallet) (verifyAddress bool) { | ||
| _, err := dcrutil.DecodeAddress(address, wallet.chainParams) | ||
| if err != nil { | ||
| return | ||
| } |
Contributor
There was a problem hiding this comment.
If an error occurs, the function must return.
Contributor
There was a problem hiding this comment.
While you don't need to return a value when using named return variables (and I agree that using a named return variable makes the tx.AddSendDestination method easier to understand), returning an explicit value in this instance makes more readable sense.
Suggested change
| func (tx *TxAuthor) AddSendDestination(address string, atomAmount int64, sendMax bool, wallet *Wallet) (verifyAddress bool) { | |
| _, err := dcrutil.DecodeAddress(address, wallet.chainParams) | |
| if err != nil { | |
| return | |
| } | |
| func (tx *TxAuthor) AddSendDestination(address string, atomAmount int64, sendMax bool, wallet *Wallet) (verifyAddress bool) { | |
| _, err := dcrutil.DecodeAddress(address, wallet.chainParams) | |
| if err != nil { | |
| return false | |
| } |
itswisdomagain
suggested changes
Mar 21, 2020
txauthor.go
Outdated
| func (tx *TxAuthor) AddSendDestination(address string, atomAmount int64, sendMax bool, wallet *Wallet) (verifyAddress bool) { | ||
| _, err := dcrutil.DecodeAddress(address, wallet.chainParams) | ||
| if err != nil { | ||
| return | ||
| } |
Contributor
There was a problem hiding this comment.
While you don't need to return a value when using named return variables (and I agree that using a named return variable makes the tx.AddSendDestination method easier to understand), returning an explicit value in this instance makes more readable sense.
Suggested change
| func (tx *TxAuthor) AddSendDestination(address string, atomAmount int64, sendMax bool, wallet *Wallet) (verifyAddress bool) { | |
| _, err := dcrutil.DecodeAddress(address, wallet.chainParams) | |
| if err != nil { | |
| return | |
| } | |
| func (tx *TxAuthor) AddSendDestination(address string, atomAmount int64, sendMax bool, wallet *Wallet) (verifyAddress bool) { | |
| _, err := dcrutil.DecodeAddress(address, wallet.chainParams) | |
| if err != nil { | |
| return false | |
| } |
itswisdomagain
suggested changes
Apr 3, 2020
ReevesAk
force-pushed
the
senddestination_verify_address
branch
from
0334931 to
efe2393
Compare
itswisdomagain
suggested changes
Apr 4, 2020
txauthor.go
Outdated
Comment on lines
+34
to
+36
| func (tx *TxAuthor) SetSourceAccount(accountNumber int32) { | ||
| tx.sendFromAccount = uint32(accountNumber) | ||
| } |
Contributor
There was a problem hiding this comment.
These changes must have crept in from your last rebase. Please review and remove all changes NOT related to the fix this PR provides. This includes changes to your go.mod and go.sum files.
ReevesAk
force-pushed
the
senddestination_verify_address
branch
from
e22ab0f to
efe2393
Compare
ReevesAk
force-pushed
the
senddestination_verify_address
branch
from
efe2393 to
afbb312
Compare
beansgum
suggested changes
Apr 12, 2020
| @@ -32,6 +32,10 @@ func (mw *MultiWallet) NewUnsignedTx(sourceWallet *Wallet, sourceAccountNumber i | |||
| } | |||
|
|
|||
| func (tx *TxAuthor) AddSendDestination(address string, atomAmount int64, sendMax bool) { | |||
Contributor
|
Close by #147 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
addDestination function modified to verify address before adding it.
This closes issue #100