Skip to content

Tiny refactor of IsRBFOptIn, avoid exception#7812

Merged
laanwj merged 1 commit intobitcoin:masterfrom
jonasschnelli:2016/04/rbf_refact
Apr 14, 2016
Merged

Tiny refactor of IsRBFOptIn, avoid exception#7812
laanwj merged 1 commit intobitcoin:masterfrom
jonasschnelli:2016/04/rbf_refact

Conversation

@jonasschnelli
Copy link
Copy Markdown
Contributor

While working with RBF in the GUI, the exception in IsRBFOptIn() (throw std::runtime_error("Cannot determine RBF opt-in signal for non-mempool transaction\n")) could be unexpected.

@maflcko
Copy link
Copy Markdown
Member

maflcko commented Apr 5, 2016

Concept ACK

@sdaftuar
Copy link
Copy Markdown
Member

sdaftuar commented Apr 5, 2016

Looks good, thanks for cleaning this up. This code is tested in listtransactions.py, which passes on my machine, so ACK (assuming travis agrees).

Comment thread src/wallet/rpcwallet.cpp Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Use a switch case?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Meh. Had that first, but the if/elseif uses less lines (just two conditions).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

In this case, I think '?' operator is the best solution here... Isn't FINAL case missing here anyway?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think the ? operator does not fit well for a non-boolean. The FINAL case is handled by the default value set at L82.

@paveljanik
Copy link
Copy Markdown
Contributor

Commit message typo: expection -> exception

@dcousens
Copy link
Copy Markdown
Contributor

dcousens commented Apr 6, 2016

utACK 1e211e2

@jonasschnelli
Copy link
Copy Markdown
Contributor Author

Force push fixed the git commit message typo.

@laanwj
Copy link
Copy Markdown
Member

laanwj commented Apr 6, 2016

utACK 4f7c959

1 similar comment
@sipa
Copy link
Copy Markdown
Member

sipa commented Apr 6, 2016

utACK 4f7c959

@laanwj laanwj merged commit 4f7c959 into bitcoin:master Apr 14, 2016
laanwj added a commit that referenced this pull request Apr 14, 2016
4f7c959 Refactor IsRBFOptIn, avoid exception (Jonas Schnelli)
codablock pushed a commit to codablock/dash that referenced this pull request Dec 20, 2017
4f7c959 Refactor IsRBFOptIn, avoid exception (Jonas Schnelli)
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants