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

feat: setup test suite #90

Merged
merged 32 commits into from
Dec 20, 2024
Merged

feat: setup test suite #90

merged 32 commits into from
Dec 20, 2024

Conversation

gusin13
Copy link
Collaborator

@gusin13 gusin13 commented Dec 8, 2024

This pr sets up the full test suite, in short - Babylon node, Bitcoin node and indexer service.

It also sets up the happy case e2e test to verify state transitions and queue events in indexer.
Pending -> Verified -> Unbonding (Early Unbonding)

@gusin13 gusin13 force-pushed the gusin13/setup-test-suite branch from 1632d4e to 44e3728 Compare December 15, 2024 10:52
@gusin13 gusin13 marked this pull request as ready for review December 16, 2024 08:14
@gusin13 gusin13 marked this pull request as draft December 16, 2024 08:14
@gusin13 gusin13 marked this pull request as ready for review December 16, 2024 08:28
Copy link
Member

@gitferry gitferry left a comment

Choose a reason for hiding this comment

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

Very nice work putting all the components together! Some comments can be resolved in a subsequent pr. No blocker

tm.WaitForFinalityProviderStored(t, ctx, fpPK.BtcPk.MarshalHex())

// Create BTC delegation without inclusion proof in Babylon node
stakingMsgTx, stakingSlashingInfo, unbondingSlashingInfo, _ := tm.CreateBTCDelegationWithoutIncl(t, fpSK)
Copy link
Member

Choose a reason for hiding this comment

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

Given that this change for expiry checker is for phase-1 transactions, we should test the case where inclusion proof is included in eoi

Copy link
Collaborator Author

@gusin13 gusin13 Dec 19, 2024

Choose a reason for hiding this comment

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

hmm right, we should test both.

maybe i am wrong but i thought web only supports preapproval flow (in phase2 as well), if the user wants to send inclusion proof he would use the staker cli instead.

i'd need to add separate test for this flow, would be better if i do in next pr (logged a ticket ref)

@jrwbabylonlab
Copy link
Collaborator

The bitcoind setup is extremely complex. Is this what's been implemented on core side across multiple services/repos?
Probably worth get in a call about this as it looks to me very hard to maintain. Some thoughts on this:

  1. If this is same code used across multiple repos. Have we considered a custom build docker image to handle this? Or altnernatly, a dedicate testing package for setting up all those things.
  2. Maybe a high-level of what we trying to achieve from bitcoind will help. I wonder if we can substitute it with wiremock instead.

@gusin13
Copy link
Collaborator Author

gusin13 commented Dec 19, 2024

The bitcoind setup is extremely complex. Is this what's been implemented on core side across multiple services/repos? Probably worth get in a call about this as it looks to me very hard to maintain. Some thoughts on this:

  1. If this is same code used across multiple repos. Have we considered a custom build docker image to handle this? Or altnernatly, a dedicate testing package for setting up all those things.
  2. Maybe a high-level of what we trying to achieve from bitcoind will help. I wonder if we can substitute it with wiremock instead.

@jrwbabylonlab hmmm 🤔 did you mean this setup here

  1. Yes this is common in multiple repos, vigilante here, babylon here, phase 1 indexer here
    I took ref from vigilante and phase 1 indexer (as vigilante also depends on babylon node and bitcoind node but phase 1 indexer depends only on bitcoind node).
    Can you elaborate a bit more on custom docker image 🤔. Do you mean instead of running all this container setup, we have a regtest docker image pushed to registry and then a simple compose file in tests can pull this. It seems like a good idea but all the repos are basically using the same convention so I simply followed 😃

  2. We are setting up here e2e/integration tests which need all the components running which the indexer needs (bitcoind node, babylon, local indexer db) to properly test things. The e2e tests would create a random delegation, insert in the babylon node, then submit cov sigs etc to babylon node, also submit incl proof to the bitcoind node and during all of this verify the state transitions in the indexer db/service.
    Mocking is ok in unit tests/fuzz etc but imo we need to spin up real services for integration/e2e (same as what babylon/vigilante/phase1 indexer does)

happy to jump on call to discuss

@jrwbabylonlab
Copy link
Collaborator

The bitcoind setup is extremely complex. Is this what's been implemented on core side across multiple services/repos? Probably worth get in a call about this as it looks to me very hard to maintain. Some thoughts on this:

  1. If this is same code used across multiple repos. Have we considered a custom build docker image to handle this? Or altnernatly, a dedicate testing package for setting up all those things.
  2. Maybe a high-level of what we trying to achieve from bitcoind will help. I wonder if we can substitute it with wiremock instead.

@jrwbabylonlab hmmm 🤔 did you mean this setup here

  1. Yes this is common in multiple repos, vigilante here, babylon here, phase 1 indexer here
    I took ref from vigilante and phase 1 indexer (as vigilante also depends on babylon node and bitcoind node but phase 1 indexer depends only on bitcoind node).
    Can you elaborate a bit more on custom docker image 🤔. Do you mean instead of running all this container setup, we have a regtest docker image pushed to registry and then a simple compose file in tests can pull this. It seems like a good idea but all the repos are basically using the same convention so I simply followed 😃
  2. We are setting up here e2e/integration tests which need all the components running which the indexer needs (bitcoind node, babylon, local indexer db) to properly test things. The e2e tests would create a random delegation, insert in the babylon node, then submit cov sigs etc to babylon node, also submit incl proof to the bitcoind node and during all of this verify the state transitions in the indexer db/service.
    Mocking is ok in unit tests/fuzz etc but imo we need to spin up real services for integration/e2e (same as what babylon/vigilante/phase1 indexer does)

happy to jump on call to discuss

"Do you mean instead of running all this container setup, we have a regtest docker image pushed to registry and then a simple compose file in tests can pull this" Correct

@gusin13 gusin13 merged commit d0cb9e9 into main Dec 20, 2024
11 checks passed
@gusin13 gusin13 deleted the gusin13/setup-test-suite branch December 20, 2024 06:35
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.

3 participants