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

wallet: return ErrNotMine for script addresses #909

Merged
merged 1 commit into from
Feb 6, 2024

Conversation

guggero
Copy link
Collaborator

@guggero guggero commented Feb 6, 2024

We recently added the ManagedTaprootScriptAddress type which is not a ManagedPubKeyAddress and therefore doesn't store the derivation info. When trying to fetch the input info for such an address we run into the case where we return nil as the error which causes issues further up in the call stack.
This commit fixes the problem by returning an actual error and not the err variable that is nil due to the previous check.

We recently added the ManagedTaprootScriptAddress type which is not a
ManagedPubKeyAddress and therefore doesn't store the derivation info.
When trying to fetch the input info for such an address we run into the
case where we return nil as the error which causes issues further up in
the call stack.
This commit fixes the problem by returning an actual error and not the
err variable that is nil due to the previous check.
@@ -135,7 +135,7 @@ func (w *Wallet) FetchInputInfo(prevOut *wire.OutPoint) (*wire.MsgTx,
}
pubKeyAddr, ok := addr.(waddrmgr.ManagedPubKeyAddress)
if !ok {
return nil, nil, nil, 0, err
return nil, nil, nil, 0, ErrNotMine
Copy link
Member

Choose a reason for hiding this comment

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

So do we need to modify this routine to support the other types? Or is this just a way to catch this case in some higher level code?

Copy link
Member

Choose a reason for hiding this comment

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

Ah also weird interaction re the err variable shadowing 🙈

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't think we need to handle the other types, really. This is mainly about returning the wrong error (it's always nil which is what breaks things at the call sites).

Copy link
Member

@Roasbeef Roasbeef left a comment

Choose a reason for hiding this comment

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

LGTM

@Roasbeef Roasbeef merged commit 1f3534b into btcsuite:master Feb 6, 2024
3 checks passed
@guggero guggero deleted the return-correct-err branch February 6, 2024 19:57
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.

2 participants