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

Proposed fix for (one type of) force_return errors. #995

Merged
merged 1 commit into from
Apr 11, 2022

Conversation

Zannick
Copy link
Collaborator

@Zannick Zannick commented Apr 8, 2022

Issue

When dealing with some orphan transactions1, some addresses aren't in the address map, and so when we look them up we get an invalid return value to indicate we didn't find it. However, we don't check for the sentinel value and instead try to look up its type (it's a boost::variant) which can assert with forced_return if the which_ field is outside the valid range.

Fix

We don't need to look up an address in the map when all we're going to do is use the key (which must be the same type per boost::variant).

Tested

Windows 10 mainnet wallet.

Footnotes

  1. This only happened for me for months-old orphan blocks, and I can't tell if the address never existed or if it got corrupted at some point.

We do not need to look up an address in the address book map
just to check the type of the key in the map, as the type
must be exactly the same as the provided destination. But if we don't
find the destination in the map, we get back `end()` which is not
a valid value and may have a `which_` field in the `boost::variant`
that can trigger the assert.
@Zannick Zannick added Component: Wallet Relating to keystore, tx creation, and balance tracking Tag: TransactionRecords The display of transaction information Component: Core App Related to the application itself. Tag: Waiting For Code Review Waiting for code review from a core developer labels Apr 8, 2022
@Zannick Zannick self-assigned this Apr 8, 2022
Copy link

@minerminer4949 minerminer4949 left a comment

Choose a reason for hiding this comment

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

ACK 61c6f10

Copy link
Collaborator

@codeofalltrades codeofalltrades left a comment

Choose a reason for hiding this comment

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

ACK 61c6f10

@codeofalltrades codeofalltrades added Code Review: Passed and removed Tag: Waiting For Code Review Waiting for code review from a core developer labels Apr 11, 2022
@codeofalltrades codeofalltrades merged commit 0354b87 into Veil-Project:master Apr 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Code Review: Passed Component: Core App Related to the application itself. Component: Wallet Relating to keystore, tx creation, and balance tracking Tag: TransactionRecords The display of transaction information
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants