search

zingolabs / zingolib

An API and test-app that exposes zcash functionality for app consumption
MIT License
15 stars 23 forks source link

removing sapling change from total change returned causes unexpected behaviour #646

Open Oscar-Pepper opened 1 year ago

Oscar-Pepper commented 1 year ago

reffering to this commit: 37868fdabc0edd2498b07e72a896bb7819189479

running the following command before and after removing sapling change from total_change_returned (https://github.com/zingolabs/zingolib/pull/645/commits/37868fdabc0edd2498b07e72a896bb7819189479#diff-fb2a3b45013be109f635e33515041d535dcbaa600df91bfb88468ee8e77d39d9R1166) cargo nextest run --features deprecations received_sapling this command runs two new tests (https://github.com/Oscar-Pepper/zingolib/blob/sapling_from_self_marked_as_change/zingocli/tests/integration_tests.rs#L3480-L3514)

BEFORE REMOVING SAPLING CHANGE:

$ cargo nextest run --features deprecations received_sapling
   Compiling zingolib v0.2.0 (/home/oscar/src/zingolib/zingolib)
   Compiling zingo-testutils v0.1.0 (/home/oscar/src/zingolib/zingo-testutils)
   Compiling zingo-cli v0.2.0 (/home/oscar/src/zingolib/zingocli)
    Finished test [optimized] target(s) in 15.41s
    Starting 2 tests across 8 binaries (75 skipped)
        FAIL [  36.992s] zingo-cli::integration_tests received_sapling_funds_not_marked_as_change

--- STDOUT:              zingo-cli::integration_tests received_sapling_funds_not_marked_as_change ---

running 1 test
test received_sapling_funds_not_marked_as_change ... FAILED

failures:

failures:
    received_sapling_funds_not_marked_as_change

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 54 filtered out; finished in 36.99s

--- STDERR:              zingo-cli::integration_tests received_sapling_funds_not_marked_as_change ---
[zingocli/tests/integration_tests.rs:3493] recipient.do_list_txsummaries().await = [
    ValueTransfer {
        block_height: BlockHeight(
            4,
        ),
        datetime: 1698770079,
        kind: Received {
            pool: Transparent,
            amount: 20000,
        },
        memos: [],
        price: None,
        txid: TxId(
            "96593e3f8cec05d1865435c5d193828609dc7a2c7978c3a7a0ace568b9d35204",
        ),
        unconfirmed: false,
    },
    ValueTransfer {
        block_height: BlockHeight(
            4,
        ),
        datetime: 1698770079,
        kind: Received {
            pool: Sapling,
            amount: 40000,
        },
        memos: [],
        price: None,
        txid: TxId(
            "96593e3f8cec05d1865435c5d193828609dc7a2c7978c3a7a0ace568b9d35204",
        ),
        unconfirmed: false,
    },
]
thread 'received_sapling_funds_not_marked_as_change' panicked at zingocli/tests/integration_tests.rs:3495:5:
explicit panic
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

        FAIL [  42.270s] zingo-cli::integration_tests received_sapling_funds_from_self_not_marked_as_change

--- STDOUT:              zingo-cli::integration_tests received_sapling_funds_from_self_not_marked_as_change ---

running 1 test
test received_sapling_funds_from_self_not_marked_as_change ... FAILED

failures:

failures:
    received_sapling_funds_from_self_not_marked_as_change

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 54 filtered out; finished in 42.26s

--- STDERR:              zingo-cli::integration_tests received_sapling_funds_from_self_not_marked_as_change ---
[zingocli/tests/integration_tests.rs:3511] recipient.do_list_txsummaries().await = [
    ValueTransfer {
        block_height: BlockHeight(
            5,
        ),
        datetime: 1698770080,
        kind: Received {
            pool: Orchard,
            amount: 70000,
        },
        memos: [],
        price: None,
        txid: TxId(
            "7a05ac92d814e2e09abda9898987ca9decad6b124e9230ef2ad51f1c3f6fca2f",
        ),
        unconfirmed: false,
    },
    ValueTransfer {
        block_height: BlockHeight(
            6,
        ),
        datetime: 1698770084,
        kind: SendToSelf,
        memos: [],
        price: None,
        txid: TxId(
            "0d7bd34110b4c90b3bd4d18ce214f5121ffd63e5183f2363c088df70aa5a938d",
        ),
        unconfirmed: false,
    },
    ValueTransfer {
        block_height: BlockHeight(
            6,
        ),
        datetime: 1698770084,
        kind: Fee {
            amount: 10000,
        },
        memos: [],
        price: None,
        txid: TxId(
            "0d7bd34110b4c90b3bd4d18ce214f5121ffd63e5183f2363c088df70aa5a938d",
        ),
        unconfirmed: false,
    },
]
thread 'received_sapling_funds_from_self_not_marked_as_change' panicked at zingocli/tests/integration_tests.rs:3513:5:
explicit panic
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

------------
     Summary [  42.271s] 2 tests run: 0 passed, 2 failed, 75 skipped
        FAIL [  42.270s] zingo-cli::integration_tests received_sapling_funds_from_self_not_marked_as_change
        FAIL [  36.992s] zingo-cli::integration_tests received_sapling_funds_not_marked_as_change
error: test run failed

AFTER REMOVING SAPLING CHANGE:

$ cargo nextest run --features deprecations received_sapling
   Compiling zingolib v0.2.0 (/home/oscar/src/zingolib/zingolib)
   Compiling zingo-testutils v0.1.0 (/home/oscar/src/zingolib/zingo-testutils)
   Compiling zingo-cli v0.2.0 (/home/oscar/src/zingolib/zingocli)
        FAIL [  37.118s] zingo-cli::integration_tests received_sapling_funds_not_marked_as_change

--- STDOUT:              zingo-cli::integration_tests received_sapling_funds_not_marked_as_change ---

running 1 test
test received_sapling_funds_not_marked_as_change ... FAILED

failures:

failures:
    received_sapling_funds_not_marked_as_change

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 54 filtered out; finished in 37.11s

--- STDERR:              zingo-cli::integration_tests received_sapling_funds_not_marked_as_change ---
[zingocli/tests/integration_tests.rs:3493] recipient.do_list_txsummaries().await = [
    ValueTransfer {
        block_height: BlockHeight(
            4,
        ),
        datetime: 1698771209,
        kind: Received {
            pool: Transparent,
            amount: 20000,
        },
        memos: [],
        price: None,
        txid: TxId(
            "5eeb890d269be73a31ebada2055696305de3f26ce6825b7405c492cff4805c87",
        ),
        unconfirmed: false,
    },
    ValueTransfer {
        block_height: BlockHeight(
            4,
        ),
        datetime: 1698771209,
        kind: Received {
            pool: Sapling,
            amount: 40000,
        },
        memos: [],
        price: None,
        txid: TxId(
            "5eeb890d269be73a31ebada2055696305de3f26ce6825b7405c492cff4805c87",
        ),
        unconfirmed: false,
    },
]
thread 'received_sapling_funds_not_marked_as_change' panicked at zingocli/tests/integration_tests.rs:3495:5:
explicit panic
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

        FAIL [  42.276s] zingo-cli::integration_tests received_sapling_funds_from_self_not_marked_as_change

--- STDOUT:              zingo-cli::integration_tests received_sapling_funds_from_self_not_marked_as_change ---

running 1 test
test received_sapling_funds_from_self_not_marked_as_change ... FAILED

failures:

failures:
    received_sapling_funds_from_self_not_marked_as_change

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 54 filtered out; finished in 42.27s

--- STDERR:              zingo-cli::integration_tests received_sapling_funds_from_self_not_marked_as_change ---
[zingocli/tests/integration_tests.rs:3511] recipient.do_list_txsummaries().await = [
    ValueTransfer {
        block_height: BlockHeight(
            5,
        ),
        datetime: 1698771210,
        kind: Received {
            pool: Orchard,
            amount: 70000,
        },
        memos: [],
        price: None,
        txid: TxId(
            "e56eecfb7db3b91c18e89ed6171359e5efe8e25ccc9f6c39a4cbd8b14fb7bb7f",
        ),
        unconfirmed: false,
    },
    ValueTransfer {
        block_height: BlockHeight(
            6,
        ),
        datetime: 1698771214,
        kind: SendToSelf,
        memos: [],
        price: None,
        txid: TxId(
            "c772beec5f24f081f170284dcc931dd0e995f10971faf28a0b0935e274f5c5fa",
        ),
        unconfirmed: false,
    },
    ValueTransfer {
        block_height: BlockHeight(
            6,
        ),
        datetime: 1698771214,
        kind: Fee {
            amount: 50000,
        },
        memos: [],
        price: None,
        txid: TxId(
            "c772beec5f24f081f170284dcc931dd0e995f10971faf28a0b0935e274f5c5fa",
        ),
        unconfirmed: false,
    },
]
thread 'received_sapling_funds_from_self_not_marked_as_change' panicked at zingocli/tests/integration_tests.rs:3512:5:
assertion `left == right` failed:
balance_report:
observed orchard: 0 expected orchard: 0
observed sapling: 40000 expected sapling: 40000
observed transpa: 20000 expected transpa: 20000

sum_of_balances: 60000 tx_summary_balance: 20000
  left: 60000
 right: 20000
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

------------
     Summary [  42.276s] 2 tests run: 0 passed, 2 failed, 75 skipped
        FAIL [  42.276s] zingo-cli::integration_tests received_sapling_funds_from_self_not_marked_as_change
        FAIL [  37.118s] zingo-cli::integration_tests received_sapling_funds_not_marked_as_change
error: test run failed
16:53:59 oscar powertower sapling_from_self_marked_as_change [ ]
~/src/zingolib exit status: 100
    Finished test [optimized] target(s) in 8.84s
    Starting 2 tests across 8 binaries (75 skipped)
     Running [ 00:00:05] [                                                                                     ]  0/2 : 2 running, 0 passed, 18 skipped

ISSUE: tx_summary_balance is incorrect received sapling funds move into FEE

Oscar-Pepper commented 1 year ago

the root cause of this is because:

  1. sapling send-to-self is marked as change
  2. fees in do_list_txsummaries are calculated by a difference between value spent and value outgoing/returned

i am investgating to see if this is related to the issue with failing tests in our feature branch implementing zip317