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

tx.author addDestination to verify address #117

Closed

Conversation

ReevesAk
Copy link

@ReevesAk ReevesAk commented Mar 11, 2020

addDestination function modified to verify address before adding it.

This closes issue #100

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
}
Copy link
Contributor

@oluwandabira oluwandabira Mar 18, 2020

Choose a reason for hiding this comment

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

If an error occurs, the function must return.

Copy link
Contributor

Choose a reason for hiding this comment

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

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
}

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
}
Copy link
Contributor

Choose a reason for hiding this comment

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

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
}

@ReevesAk ReevesAk force-pushed the senddestination_verify_address branch from 0334931 to efe2393 Compare April 3, 2020 20:29
txauthor.go Outdated
Comment on lines 34 to 36
func (tx *TxAuthor) SetSourceAccount(accountNumber int32) {
tx.sendFromAccount = uint32(accountNumber)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

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 ReevesAk force-pushed the senddestination_verify_address branch from e22ab0f to efe2393 Compare April 7, 2020 09:00
@ReevesAk ReevesAk force-pushed the senddestination_verify_address branch from efe2393 to afbb312 Compare April 8, 2020 17:29
@@ -32,6 +32,10 @@ func (mw *MultiWallet) NewUnsignedTx(sourceWallet *Wallet, sourceAccountNumber i
}

func (tx *TxAuthor) AddSendDestination(address string, atomAmount int64, sendMax bool) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Address this review and run go mod tidy.

@beansgum
Copy link
Contributor

Close by #147

@beansgum beansgum closed this Jul 25, 2020
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.

4 participants