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

feat: console and FFI should have setting to not choose outputs that reveal the address #4403 #4516

Conversation

agubarev
Copy link
Contributor

Description

#4403
Wallet (console and FFI) should have setting to not choose outputs that reveal the address #4403

Motivation and Context

Problem
Wallets currently will choose the best outputs as inputs when spending, however since a lurking base node can generate a transaction graph of inputs to outputs with relative ease, a wallet may reveal its transaction history by including a (non-stealth address) one-sided payment.

For example, an attacker wishing to know the transaction graph of a public key PK_Alice can send a one-sided payment to PK_Alice using the Tariscript Push(PK_Alice). At some point, Alice's wallet spends this transaction without realizing it.

Possible solution
Could change the wallet to have a config setting, to not include one-sided payments by default when spending

How Has This Been Tested?

@agubarev agubarev changed the title feature: wallet (console and FFI) should have setting to not choose outputs that reveal the address #4403 feat: wallet (console and FFI) should have setting to not choose outputs that reveal the address #4403 Aug 23, 2022
…ontrols whether outputs received via a simple one-sided transaction are automatically selected by `select_utxos` when spending

added `source` field to `outputs` table

Signed-off-by: Andrei Gubarev <agubarev@users.noreply.github.com>
Signed-off-by: Andrei Gubarev <agubarev@users.noreply.github.com>
Signed-off-by: Andrei Gubarev <agubarev@users.noreply.github.com>
Signed-off-by: Andrei Gubarev <agubarev@users.noreply.github.com>
@agubarev agubarev force-pushed the issue-4403-ignore-onesided-outputs branch from 1d37cf7 to c827735 Compare August 26, 2022 10:16
@agubarev agubarev changed the title feat: wallet (console and FFI) should have setting to not choose outputs that reveal the address #4403 feat(wallet) console and FFI should have setting to not choose outputs that reveal the address #4403 Aug 26, 2022
@agubarev agubarev changed the title feat(wallet) console and FFI should have setting to not choose outputs that reveal the address #4403 feat(wallet): console and FFI should have setting to not choose outputs that reveal the address #4403 Aug 26, 2022
@agubarev agubarev changed the title feat(wallet): console and FFI should have setting to not choose outputs that reveal the address #4403 feat: console and FFI should have setting to not choose outputs that reveal the address #4403 Aug 26, 2022
@agubarev agubarev marked this pull request as ready for review August 29, 2022 14:42
Copy link
Member

@sdbondi sdbondi left a comment

Choose a reason for hiding this comment

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

LGTM - just the eprintln to remove (this will bork the console wallet)

@@ -200,13 +202,22 @@ impl OutputSql {
.filter(outputs::status.eq(OutputStatus::Unspent as i32))
.order_by(outputs::spending_priority.desc());

eprintln!(
Copy link
Member

Choose a reason for hiding this comment

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

Debug print

agubarev and others added 2 commits August 30, 2022 13:45
Signed-off-by: Andrei Gubarev <agubarev@users.noreply.github.com>
Copy link
Collaborator

@SWvheerden SWvheerden left a comment

Choose a reason for hiding this comment

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

Looks good, just one nit

common/config/presets/d_console_wallet.toml Outdated Show resolved Hide resolved
agubarev and others added 5 commits August 30, 2022 18:17
Co-authored-by: SW van Heerden <swvheerden@gmail.com>
…urce.rs

Co-authored-by: Stan Bondi <sdbondi@users.noreply.github.com>
Signed-off-by: Andrei Gubarev <agubarev@users.noreply.github.com>
Signed-off-by: Andrei Gubarev <agubarev@users.noreply.github.com>
@@ -151,6 +152,7 @@ where
&self.rewind_data,
None,
Some(proof),
OutputSource::Recovered,
Copy link
Member

Choose a reason for hiding this comment

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

Even when recovered, the output should be marked according to its script (one-sided, stealth, standard, etc.)

sdbondi
sdbondi previously approved these changes Aug 31, 2022
Copy link
Member

@sdbondi sdbondi left a comment

Choose a reason for hiding this comment

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

LGTM, but it looks like recovered outputs are not categorized. This can be done in another PR.

stringhandler
stringhandler previously approved these changes Aug 31, 2022
base_layer/wallet/src/output_manager_service/service.rs Outdated Show resolved Hide resolved
@agubarev agubarev dismissed stale reviews from stringhandler and sdbondi via 2aa7eed August 31, 2022 08:15
Co-authored-by: stringhandler <stringhandler@gmail.com>
@stringhandler stringhandler merged commit 17bb64e into tari-project:development Aug 31, 2022
jorgeantonio21 pushed a commit to jorgeantonio21/tari that referenced this pull request Aug 31, 2022
…reveal the address tari-project#4403 (tari-project#4516)

Description
---
tari-project#4403
Wallet (console and FFI) should have setting to not choose outputs that reveal the address tari-project#4403

Motivation and Context
---
Problem
Wallets currently will choose the best outputs as inputs when spending, however since a lurking base node can generate a transaction graph of inputs to outputs with relative ease, a wallet may reveal its transaction history by including a (non-stealth address) one-sided payment.

For example, an attacker wishing to know the transaction graph of a public key PK_Alice can send a one-sided payment to PK_Alice using the Tariscript Push(PK_Alice). At some point, Alice's wallet spends this transaction without realizing it.

Possible solution
Could change the wallet to have a config setting, to not include one-sided payments by default when spending

How Has This Been Tested?
---
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.

None yet

4 participants