Skip to content

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

Merged
codeofalltrades merged 1 commit intoVeil-Project:masterfrom
Zannick:forced_return
Apr 11, 2022
Merged

Proposed fix for (one type of) force_return errors.#995
codeofalltrades merged 1 commit intoVeil-Project:masterfrom
Zannick:forced_return

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