Improve CWallet::IsFromMe for positive results #12299

pull promag wants to merge 1 commits into bitcoin:master from promag:2018-01-isfromme changing 1 files +4 −1

promag commented at 12:34 AM on January 30, 2018: member

Trivial change that avoids computing the transaction debit by early returning when an input is debit.
[wallet] Improve CWallet::IsFromMe for positive results ae7c363529
fanquake added the label Wallet on Jan 30, 2018
laanwj commented at 9:34 AM on January 30, 2018: member

Does have some overlap with #9167, I think
promag commented at 9:55 AM on January 30, 2018: member

Does have some overlap with #9167, I think

Thanks @laanwj I'll take look, but that one looks unmaintained.
laanwj commented at 10:04 AM on January 30, 2018: member

Thanks @laanwj I'll take look, but that one looks unmaintained.

It does have ACKs already, so it's just a matter of pushing it over the finish.
laanwj commented at 8:57 AM on January 31, 2018: member

Like for #12297, we need some load test or benchmark or profiling to evaluate this change.
ryanofsky commented at 8:30 PM on February 5, 2018: member

utACK ae7c3635293930c804248954ede4bf8353431ed9
promag commented at 1:30 AM on February 9, 2018: member

Self review, this is incomplete, missing refactor to CWalletTx::IsFromMe.

Does have some overlap with #9167, I think @laanwj I don't think so.

we need some load test or benchmark or profiling to evaluate this change.

CWallet::IsFromMe is only called in CWallet::AddToWalletIfInvolvingMe: https://github.com/bitcoin/bitcoin/blob/67447ba06057b8e83f962c82491d2fe6c5211f50/src/wallet/wallet.cpp#L1039

I only claim improvement for positive results 😄 which should be near to none considering the blockchain.

CWallet::IsMine(tx) already early returns when an input is mine. Sounds reasonable to do the same in IsFromMe(tx).
Sjors commented at 1:30 PM on May 30, 2018: member

Self review, this is incomplete, missing refactor to CWalletTx::IsFromMe.

Any plans to update this?
promag commented at 1:33 PM on May 30, 2018: member

@Sjors sure, will get back to this after other active PRs.
DrahtBot closed this on Jul 20, 2018
DrahtBot commented at 8:29 PM on July 20, 2018: member

The last travis run for this pull request was 171 days ago and is thus outdated. To trigger a fresh travis build, this pull request should be closed and re-opened.
DrahtBot reopened this on Jul 20, 2018
meshcollider commented at 11:23 AM on November 11, 2018: contributor

#12508 replaces the IsFromMe function making this redundant if that is merged, correct?
promag commented at 3:47 PM on November 26, 2018: member

Self review, this is incomplete, missing refactor to CWalletTx::IsFromMe.

Any plans to update this? @Sjors actually it's not necessary as CWalletTx caches the value.

#12508 replaces the IsFromMe function making this redundant if that is merged, correct? @MeshCollider that was closed.

Anyway, this only improves performance for transaction with wallet inputs upfront, which avoids computing remaining input debits. @laanwj @MarcoFalke this change is really simple but not worth the effort to benchmark. Feel free to close if it's not mergeable.
MarcoFalke commented at 4:00 PM on November 26, 2018: member

Slight NACK because it makes the already large wallet.cpp bigger by three lines, but the code appears correct.
promag closed this on Nov 26, 2018
promag deleted the branch on Nov 26, 2018
MarcoFalke locked this on Sep 8, 2021

Contributors

Labels