Skip to content

Conversation

yyforyongyu
Copy link
Collaborator

To decide whether an outpoint belongs to the wallet or not, we need to further check that the specified output index is indeed a credit paid to us.

Copy link
Contributor

@ziggie1984 ziggie1984 left a comment

Choose a reason for hiding this comment

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

Thank you for the fix 🙏


// Create a tx that has two outputs - output1 belongs to the wallet,
// output2 is external.
output1 := wire.NewTxOut(100000, p2shAddr)
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm why do they they pay to the same addr thought therefore we created addr above ?

Copy link
Contributor

Choose a reason for hiding this comment

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

doesn't matter but still better to have a proper logic in.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

they are dummy values - techinically they should be the same so we know the result doesn't depend on the pkscript, but again I don't think it matters.

name: "no credit found",
prevOut: &wire.OutPoint{
Hash: tx.TxHash(),
Index: 1000,
Copy link
Contributor

Choose a reason for hiding this comment

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

wrong index probably you wanted to use 1 here ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

correct

Hash: tx.TxHash(),
Index: 1000,
},
errExpected: "invalid output index",
Copy link
Contributor

@ziggie1984 ziggie1984 May 23, 2025

Choose a reason for hiding this comment

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

wrong error (when using with the right index (index 1) ?

Copy link
Contributor

Choose a reason for hiding this comment

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

missing the success case as well ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

see TestFetchOutpointInfo

To decide whether an outpoint belongs to the wallet or not, we need to
further check that the specified output index is indeed a credit paid to
us.
Copy link
Contributor

@ziggie1984 ziggie1984 left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Collaborator

@guggero guggero left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the fix!

@guggero guggero merged commit 9986a2a into btcsuite:master May 26, 2025
3 checks passed
@yyforyongyu yyforyongyu deleted the fix-fetchop branch May 26, 2025 09:12
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.

3 participants