Additional checks for input existence in transactions #2405

piotr-iohk · 2020-12-18T12:55:11Z

Issue Number

It turns out there are no checks for that.
A bit related to #2327.

Overview

2605a9f
Additional checks for inputs in incoming and outgoing transactions in integration tests
1247f9a
Additional checks for inputs in join/quit pool transactions
bd2916b
remove redundant cases & fix fixture wallet sharing common funding UTxOs
It's a bit fucked up. Since the scenario is using fixtureWallet, the
UtxO from the wallet comes from one of the fixture transaction made
when initializing the cluster. There are many pre-funded fixture
wallets with 10 UTxO, but in order not to spend 10 days funding them
all, the cluster generate pretty large transaction with ~100
outputs. So a single fixture transaction funds about 10 fixture
wallets.

Now it means that, a particular fixture wallet will know the value
of all inputs of its funding fixture transaction and therefore, when
picking two fixtureWallet one after the other, it may likely happen
that both wallets are able to resolve inputs of any UTxO coming from
their original fixture transaction.

I fixed it by simply using fixtureWalletWith [...] which generates
completly new and uncorrelated UTxOs.
cdf556a
Add input check for multi-address pending tx

Comments

piotr-iohk · 2020-12-18T12:56:20Z

lib/core-integration/src/Test/Integration/Scenario/API/Shelley/StakePools.hs

+            -- all tx inputs have address and amount
+            , expectField #inputs $ \inputs' -> do
+                let addrAmt = (fmap source inputs')
+                addrAmt `shouldSatisfy` (all isJust)


Test fails here... it turns out that for delegation inputs are not there in the output of the join/quit transaction.

@KtorZ as far as your simplifications to the checks, I see you have removed this one and also the one from multi-output transactions. I'd argue that these are redundant, especially that one above was failing before (not because of fixtureWallet thing). It is OK now (after #2370 I suppose) but still worth checking.

Whoops, sorry, you didn't remove that. It was the commit diff that mislead me 😅
Anyway... I have added small additional check to multi-output tx in cdf556a.

piotr-iohk · 2020-12-18T12:57:38Z

lib/core-integration/src/Test/Integration/Scenario/API/Shelley/Transactions.hs

+                -- as the tx is incoming
+                , expectField #inputs $ \inputs' -> do
+                    let addrAmt2 = (fmap source inputs')
+                    addrAmt2 `shouldSatisfy` (all isNothing)


Fails here 🤔 ... inputs are there on incoming transaction on fixtureWallet

I can confirm this also on shelley-test-cluster. So it seems that:

in case of transaction fixtureWalletSrc -> fixtureWalletDest there are resolved inputs visible on incoming transaction on fixtureWalletDest.

in case of transaction emptyWallet -> fixtureWallet there are NO resolved inputs visible on incoming transaction on fixtureWallet

Could this be due to fixtureWallet share the same key or something, so they are able to resolve the inputs in case of transaction between them?

Yeah it's a bit fucked up. Since the scenario is using fixtureWallet, the UtxO from the wallet comes from one of the fixture transaction made when initializing the cluster. There are many pre-funded fixture wallets with 10 UTxO, but in order not to spend 10 days funding them all, the cluster generate pretty large transaction with ~100 outputs. So a single fixture transaction funds about 10 fixture wallets.

Now it means that, a particular fixture wallet will know the value of all inputs of its funding fixture transaction and therefore, when picking two fixtureWallet one after the other, it may likely happen that both wallets are able to resolve inputs of any UTxO coming from their original fixture transaction.

I fixed it by simply using fixtureWalletWith [...] which generates completly new and uncorrelated UTxOs.

piotr-iohk · 2020-12-18T12:58:15Z

lib/core-integration/src/Test/Integration/Scenario/API/Shelley/Transactions.hs

+                -- as the tx is incoming
+                , expectField #inputs $ \inputs' -> do
+                    let addrAmt2 = (fmap source inputs')
+                    addrAmt2 `shouldSatisfy` (all isNothing)


Doesn't fail here... The difference is that the destination wallet is emptyWallet.

… integration tests

It's a bit fucked up. Since the scenario is using fixtureWallet, the UtxO from the wallet comes from one of the fixture transaction made when initializing the cluster. There are many pre-funded fixture wallets with 10 UTxO, but in order not to spend 10 days funding them all, the cluster generate pretty large transaction with ~100 outputs. So a single fixture transaction funds about 10 fixture wallets. Now it means that, a particular fixture wallet will know the value of all inputs of its funding fixture transaction and therefore, when picking two fixtureWallet one after the other, it may likely happen that both wallets are able to resolve inputs of any UTxO coming from their original fixture transaction. I fixed it by simply using fixtureWalletWith [...] which generates completly new and uncorrelated UTxOs.

KtorZ

Reviewed it, fixed it and removed some redundant cases 👍

KtorZ · 2020-12-31T13:55:11Z

bors merge

2370: [ADP-500] Change addresses are not listed as soon as a transaction is no longer pending r=KtorZ a=hasufell # Issue Number ADP-500 # Overview - [x] created an integration test that checks the accepctance criteria and fails on the current branch - [x] fixed the code in `knownAddresses` to also return all used change addresses # Comments ## Questions Do we need to communicate this to API users? 2405: Additional checks for input existence in transactions r=KtorZ a=piotr-iohk # Issue Number It turns out there are no checks for that. A bit related to #2327. # Overview - 1c7215b Additional checks for inputs in incoming and outgoing transactions in integration tests - cabc8b9 Additional checks for inputs in join/quit pool transactions # Comments   Co-authored-by: Julian Ospald <[email protected]> Co-authored-by: KtorZ <[email protected]> Co-authored-by: Piotr Stachyra <[email protected]>

iohk-bors · 2020-12-31T14:29:20Z

Build failed (retrying...):

ci/hydra-build:required

  src/Test/Integration/Scenario/API/Shelley/StakePools.hs:199:5: 
  2) API Specifications, SHELLEY_STAKE_POOLS, STAKE_POOLS_JOIN_01 - Cannot join existent stakepool with wrong password
       uncaught exception: RequestException
       DecodeFailure "{\"code\":\"network_unreachable\",\"message\":\"The node backend is unreachable at the moment. Trying again in a bit might work.\"}"

#2320

2405: Additional checks for input existence in transactions r=KtorZ a=piotr-iohk # Issue Number It turns out there are no checks for that. A bit related to #2327. # Overview - 1c7215b Additional checks for inputs in incoming and outgoing transactions in integration tests - cabc8b9 Additional checks for inputs in join/quit pool transactions # Comments   Co-authored-by: Piotr Stachyra <[email protected]> Co-authored-by: KtorZ <[email protected]>

iohk-bors · 2020-12-31T15:54:51Z

Build failed:

buildkite/cardano-wallet

      src/Test/Integration/Scenario/API/Shelley/StakePools.hs:420:5:
            1) API Specifications, SHELLEY_STAKE_POOLS, STAKE_POOLS_JOIN_02 - Cannot join already joined stake pool
                       uncaught exception: RequestException
                                  DecodeFailure "{\"code\":\"network_unreachable\",\"message\":\"The node backend is unreachable at the moment. Trying again in a bit might work.\"}"

#2320

KtorZ · 2020-12-31T16:06:34Z

bors merge

2405: Additional checks for input existence in transactions r=KtorZ a=piotr-iohk # Issue Number It turns out there are no checks for that. A bit related to #2327. # Overview - 1c7215b Additional checks for inputs in incoming and outgoing transactions in integration tests - cabc8b9 Additional checks for inputs in join/quit pool transactions # Comments   Co-authored-by: Piotr Stachyra <[email protected]> Co-authored-by: KtorZ <[email protected]>

iohk-bors · 2020-12-31T16:37:02Z

Build failed:

ci/hydra-build:required

Failures:

  src/Test/Integration/Scenario/API/Shelley/StakePools.hs:506:5:
  1) API Specifications, SHELLEY_STAKE_POOLS, STAKE_POOLS_JOIN_01 - Can rejoin another stakepool
       uncaught exception: RequestException
       DecodeFailure "{\"code\":\"network_unreachable\",\"message\":\"The node backend is unreachable at the moment. Trying again in a bit might work.\"}"

  To rerun use: --match "/API Specifications/SHELLEY_STAKE_POOLS/STAKE_POOLS_JOIN_01 - Can rejoin another stakepool/"

Randomized with seed 1875000912

#2320

piotr-iohk · 2021-01-05T13:08:32Z

bors r+

iohk-bors · 2021-01-05T14:10:30Z

Build succeeded:

piotr-iohk added the IMPROVEMENT Mark a PR as an improvement, for auto-generated CHANGELOG label Dec 18, 2020

piotr-iohk self-assigned this Dec 18, 2020

piotr-iohk commented Dec 18, 2020

View reviewed changes

piotr-iohk force-pushed the piotr/check-inputs-in-tx-tests branch from 37ee67f to cabc8b9 Compare December 18, 2020 14:31

Piotr Stachyra and others added 3 commits December 31, 2020 14:14

Additional checks for inputs in incoming and outgoing transactions in…

2605a9f

… integration tests

Additional checks for inputs in join/quit pool transactions

1247f9a

KtorZ force-pushed the piotr/check-inputs-in-tx-tests branch from cabc8b9 to bd2916b Compare December 31, 2020 13:54

KtorZ approved these changes Dec 31, 2020

View reviewed changes

Add input check for multi-address pending tx

cdf556a

iohk-bors bot merged commit 37ff0b2 into master Jan 5, 2021

iohk-bors bot deleted the piotr/check-inputs-in-tx-tests branch January 5, 2021 14:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Additional checks for input existence in transactions #2405

Additional checks for input existence in transactions #2405

piotr-iohk commented Dec 18, 2020 •

edited

Loading

piotr-iohk Dec 18, 2020

piotr-iohk Jan 5, 2021

piotr-iohk Jan 5, 2021

piotr-iohk Dec 18, 2020

piotr-iohk Dec 22, 2020

KtorZ Dec 31, 2020

piotr-iohk Dec 18, 2020

KtorZ left a comment

KtorZ commented Dec 31, 2020

iohk-bors bot commented Dec 31, 2020 •

edited by KtorZ

Loading

iohk-bors bot commented Dec 31, 2020 •

edited by KtorZ

Loading

KtorZ commented Dec 31, 2020

iohk-bors bot commented Dec 31, 2020 •

edited by piotr-iohk

Loading

piotr-iohk commented Jan 5, 2021

iohk-bors bot commented Jan 5, 2021

Additional checks for input existence in transactions #2405

Additional checks for input existence in transactions #2405

Conversation

piotr-iohk commented Dec 18, 2020 • edited Loading

Issue Number

Overview

Comments

piotr-iohk Dec 18, 2020

Choose a reason for hiding this comment

piotr-iohk Jan 5, 2021

Choose a reason for hiding this comment

piotr-iohk Jan 5, 2021

Choose a reason for hiding this comment

piotr-iohk Dec 18, 2020

Choose a reason for hiding this comment

piotr-iohk Dec 22, 2020

Choose a reason for hiding this comment

KtorZ Dec 31, 2020

Choose a reason for hiding this comment

piotr-iohk Dec 18, 2020

Choose a reason for hiding this comment

KtorZ left a comment

Choose a reason for hiding this comment

KtorZ commented Dec 31, 2020

iohk-bors bot commented Dec 31, 2020 • edited by KtorZ Loading

iohk-bors bot commented Dec 31, 2020 • edited by KtorZ Loading

KtorZ commented Dec 31, 2020

iohk-bors bot commented Dec 31, 2020 • edited by piotr-iohk Loading

piotr-iohk commented Jan 5, 2021

iohk-bors bot commented Jan 5, 2021

piotr-iohk commented Dec 18, 2020 •

edited

Loading

iohk-bors bot commented Dec 31, 2020 •

edited by KtorZ

Loading

iohk-bors bot commented Dec 31, 2020 •

edited by KtorZ

Loading

iohk-bors bot commented Dec 31, 2020 •

edited by piotr-iohk

Loading