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

tests: Bouncer test for broker level screening #5377

Merged
merged 19 commits into from
Nov 6, 2024

Conversation

Janislav
Copy link
Contributor

@Janislav Janislav commented Nov 4, 2024

Pull Request

Closes: PRO-1746

Checklist

Please conduct a thorough self-review before opening the PR.

  • I am confident that the code works.
  • I have written sufficient tests.
  • I have written and tested required migrations.
  • I have updated documentation where appropriate.

Summary

Adds a bouncer tests for testing broker level screening on-chain functionality. We are covering the follwing scenarios in this test:

  1. No boost and early tx report -> Tainted tx is reported early and the swap is refunded.
  2. Boost and early tx report -> Tainted tx is reported early and the swap is refunded.
  3. Boost and late tx report -> Tainted tx is reported late and the swap is not refunded.

Non-Breaking changes

If this PR includes non-breaking changes, select the non-breaking label. On merge, CI will automatically cherry-pick the commit to a PR against the release branch.

Copy link

codecov bot commented Nov 4, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 71%. Comparing base (5180af2) to head (2e17ff5).
Report is 6 commits behind head on main.

Additional details and impacted files
@@          Coverage Diff           @@
##            main   #5377    +/-   ##
======================================
- Coverage     72%     71%    -0%     
======================================
  Files        495     495            
  Lines      86276   86345    +69     
  Branches   86276   86345    +69     
======================================
- Hits       61717   61650    -67     
- Misses     21783   21893   +110     
- Partials    2776    2802    +26     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@kylezs kylezs self-requested a review November 4, 2024 12:11
@Janislav Janislav marked this pull request as ready for review November 4, 2024 15:25
bouncer/shared/send_btc.ts Show resolved Hide resolved
bouncer/tests/all_concurrent_tests.ts Outdated Show resolved Hide resolved
bouncer/tests/broker_level_screening.ts Outdated Show resolved Hide resolved
bouncer/tests/broker_level_screening.ts Outdated Show resolved Hide resolved
bouncer/tests/broker_level_screening.ts Outdated Show resolved Hide resolved
bouncer/tests/broker_level_screening.ts Outdated Show resolved Hide resolved
true,
btcRefundAddress,
0,
MILLI_SECS_PER_BLOCK * 2,
Copy link
Contributor

Choose a reason for hiding this comment

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

?

Suggested change
MILLI_SECS_PER_BLOCK * 2,
MILLI_SECS_PER_BLOCK * BLOCKS_TO_WAIT,

also these are all the same. if we need to wait the same duration, then would just factor that out

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For now. But maybe not forever. It's likely to change if we maybe want to extend the test suite, modify it or play with different values to see what the results are. We usually define this kind of constants in test suites. Don't see why it should be bad...

Copy link
Contributor

Choose a reason for hiding this comment

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

just unnecessary complexity atm, we can add the parameterisation when we need it (which might be never) - i won't block the the PR because of it, but yeah, is unnecessary

bouncer/tests/all_concurrent_tests.ts Outdated Show resolved Hide resolved
@Janislav Janislav added this pull request to the merge queue Nov 6, 2024
Merged via the queue into main with commit 2e11aed Nov 6, 2024
48 of 49 checks passed
@Janislav Janislav deleted the tests/pro-1746/bls-testing branch November 6, 2024 12:35
dandanlen pushed a commit that referenced this pull request Nov 6, 2024
* tests: Added first draft of bouncer test

* chore: added current work in progress

* chore: added current status

* chore: restored

* chore: added current status

* chore: added current state

* tests: integrated broker level screening test

* chore: Tidy up

* chore: eslint / prettier

* chore: added some comments + smallish refactors

* chore: parse unknown to string type

* chore: Make test verification more explicit

* chore: addressed comments

* chore: removed from runAllConcurrentTests

* feature: added mode to run some tests concurrent in localnet

* chore: Fixed var name
syan095 added a commit that referenced this pull request Nov 7, 2024
* origin/main:
  feat: add version and affiliates to UTXO encoding (#5385)
  chore: bump all versions to 1.8.0 and remove old migrations. (#5327)
  feat: update custom_rpc, runtime_api and broker api for broker level screening (#5341)
  tests: Bouncer test for broker level screening (#5377)
  Feat: Private Broker Channel Witnessing (#5383)

# Conflicts:
#	state-chain/runtime/src/lib.rs
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.

2 participants