Repository navigation
Conversation
…eamMW#1764) Add wallet_api_test cases that run the real V6Api handlers against a real WalletDB with txs and addresses tagged for app A, app B and none: - tx_list / tx_status / tx_cancel / tx_delete / export_payment_proof: an app sees and acts only on its own txs, a normal client on all - addr_list / create_address / edit_address / delete_address: an app sees and changes only the addresses it created They found a bug: addr_list compared the address category with the app *name*, but addresses are tagged with the app *id* (every other check uses the id; typo from f7dcfab). So an app's addr_list never returned the addresses it created. Compare with getAppId().
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fix: tests that apps can only see and change their own txs and addresses, run against a real wallet DB (#1764). The tests found a bug in
addr_listfor apps, which is fixed here too.Why: the API tests only checked that some methods are blocked for apps. The per-app filtering (
checkTxAccessRights, address categories) had no tests, because they don't use a DB. The new tests found thataddr_listcompared the address category with the app name, while addresses are tagged with the app id. Every other check uses the id; the typo dates from f7dcfab (v6.1 API). So an app'saddr_listnever returned the addresses it created.How to see the bug (master): apply only the test file and run it:
Improvement
addr_list own:truecalled by app A, which created addrA[][addrA](and not app B's or untagged addresses)Tests (a
V6Apisubclass over a real SQLiteWalletDB; no node, no ports):TestAppsTxAccess: txs tagged A, B and untagged.tx_list: normal client sees all, app A only A, app B only B, an unknown app none.tx_status/tx_cancel/tx_deleteon a foreign tx answer like an unknown tx, and nothing is deleted.TestAppsPaymentProofAccess:export_payment_proofon a foreign tx gives the same error as a missing tx.TestAppsAddressAccess:addr_listper app;create_addressby an app is tagged with its id;edit_address/delete_addresson foreign addresses are refused.Testing:
wallet_api_testpasses. Negative controls: master'saddr_listgives 3 failures; makingcheckTxAccessRightsalways allow gives 17 failures. CRLF line endings are kept. Fork CI (running): https://github.com/vsnation/beam/actions/runs/Fixes #1764