Skip to content

Fix chain index query filtering #153

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

Merged
merged 11 commits into from
Sep 20, 2022
Merged

Conversation

mikekeke
Copy link
Collaborator

@mikekeke mikekeke commented Sep 15, 2022

This PR:

  • Adds handling of missing cases when collateral should not be returned by chain-index responses
  • Adds node query effect like UtxosAt but with filtering - UtxosAtExcluding
  • Enables other collateral tests (they were not included to suite for some reason)
  • Fixes some collateral-affected tests

- added: wip filtering for chain-index
- added: filtering for node query
- failing: test suite - see "FIXME: redeem:"
- move c-indexquery adjustment to callers side
- added: c-index requet-to-response matching check
- fixed: tests
@mikekeke mikekeke added the bug Something isn't working label Sep 15, 2022
- use middleware to adjust c-index queries
- added: test for checking handling in Contract
- fixed: mismatched c-index response
- added: mock implementation for utxoRefMembership and test for it
- fixed/adjusted: rest collateral tests
@mikekeke mikekeke marked this pull request as ready for review September 19, 2022 10:20
@mikekeke mikekeke requested a review from zmrocze September 19, 2022 10:20
collateralUtxo <- readCollateralUtxo contractEnv
let removeCollateral :: TxosResponse -> TxosResponse
removeCollateral (TxosResponse page) = TxosResponse (removeCollateralFromPage collateralUtxo page)
removeCollateral
Copy link
Collaborator Author

@mikekeke mikekeke Sep 19, 2022

Choose a reason for hiding this comment

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

Removing now handled by withCollateralHandling in handlePABEffect and runPABEffectPure

collateralUtxo <- readCollateralUtxo contractEnv
let removeCollateral :: UtxosResponse -> UtxosResponse
removeCollateral (UtxosResponse tip page) = UtxosResponse tip (removeCollateralFromPage collateralUtxo page)
removeCollateral
Copy link
Collaborator Author

@mikekeke mikekeke Sep 19, 2022

Choose a reason for hiding this comment

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

Removing now handled by withCollateralHandling in handlePABEffect and runPABEffectPure

let newPage = removeCollateralFromPage mc (page utxosResp)
in UtxoSetWithCurrencyResponse $ utxosResp {page = newPage}
-- adjustment based on request
(UtxoSetMembership oref, UtxoSetMembershipResponse (IsUtxoResponse ct isU)) ->
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It is not too optimal to make request when we know already that oref is collateral, but I thought this way there will single function to adjust both real and mock interpretation, reducing the risk to get only mock implementation working correctly.

Any suggestions on how to make it better are welcome.

Copy link
Collaborator Author

@mikekeke mikekeke Sep 20, 2022

Choose a reason for hiding this comment

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

Actually, filtering by query can be made in withCollateralHandling and just do not make request and don't call adjustChainIndexResponse if not required 🤔

And adjustChainIndexResponse can go to where part of withCollateralHandling

Copy link
Contributor

@zmrocze zmrocze 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

@zmrocze zmrocze merged commit 857ec74 into master Sep 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants