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

test(tendermint): migrate to local/offline containerized testnets #2128

Merged
merged 43 commits into from
Jul 5, 2024

Conversation

onur-ozkan
Copy link
Member

@onur-ozkan onur-ozkan commented May 28, 2024

This PR refactors the tendermint integration tests as follows:

  • Run a local ATOM testnet network inside a container and use it in the integration tests.
  • Run a local NUCLEUS testnet network inside a container and use it in the integration tests.
  • Run a local IBC relayer node inside a container connected to both the ATOM and NUCLEUS local testnets, and use it in the integration tests.
  • Add test coverage for Tendermint - ETH swap.

The files inside the .docker/container-state directory ensure that the testnet networks start with a fixed state for consistent test results.

Signed-off-by: onur-ozkan <work@onurozkan.dev>
@onur-ozkan onur-ozkan added Refactoring in progress Changes will be made from the author labels May 28, 2024
Signed-off-by: onur-ozkan <work@onurozkan.dev>
Signed-off-by: onur-ozkan <work@onurozkan.dev>
Signed-off-by: onur-ozkan <work@onurozkan.dev>
@onur-ozkan onur-ozkan force-pushed the offline-tendermint-testnets branch from 6a8acb7 to 335589f Compare May 29, 2024 14:25
Signed-off-by: onur-ozkan <work@onurozkan.dev>
Signed-off-by: onur-ozkan <work@onurozkan.dev>
Signed-off-by: onur-ozkan <work@onurozkan.dev>
Signed-off-by: onur-ozkan <work@onurozkan.dev>
Signed-off-by: onur-ozkan <work@onurozkan.dev>
Signed-off-by: onur-ozkan <work@onurozkan.dev>
Signed-off-by: onur-ozkan <work@onurozkan.dev>
Signed-off-by: onur-ozkan <work@onurozkan.dev>
Signed-off-by: onur-ozkan <work@onurozkan.dev>
Signed-off-by: onur-ozkan <work@onurozkan.dev>
Signed-off-by: onur-ozkan <work@onurozkan.dev>
Signed-off-by: onur-ozkan <work@onurozkan.dev>
Signed-off-by: onur-ozkan <work@onurozkan.dev>
Signed-off-by: onur-ozkan <work@onurozkan.dev>
Signed-off-by: onur-ozkan <work@onurozkan.dev>
Signed-off-by: onur-ozkan <work@onurozkan.dev>
@onur-ozkan onur-ozkan force-pushed the offline-tendermint-testnets branch from 56784a2 to a19237b Compare June 6, 2024 05:53
Signed-off-by: onur-ozkan <work@onurozkan.dev>
@onur-ozkan onur-ozkan force-pushed the offline-tendermint-testnets branch from ae6878a to af75d4e Compare June 6, 2024 14:51
Signed-off-by: onur-ozkan <work@onurozkan.dev>
Signed-off-by: onur-ozkan <work@onurozkan.dev>
@onur-ozkan onur-ozkan force-pushed the offline-tendermint-testnets branch 4 times, most recently from 4fffa84 to 6696109 Compare June 12, 2024 06:35
Signed-off-by: onur-ozkan <work@onurozkan.dev>
@onur-ozkan onur-ozkan added under review and removed in progress Changes will be made from the author labels Jun 12, 2024
Signed-off-by: onur-ozkan <work@onurozkan.dev>
Copy link
Collaborator

@mariocynicys mariocynicys left a comment

Choose a reason for hiding this comment

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

Awesome! 🎉

First review iter.

mm2src/mm2_main/tests/docker_tests_main.rs Outdated Show resolved Hide resolved
mm2src/mm2_main/tests/docker_tests/tendermint_tests.rs Outdated Show resolved Hide resolved
mm2src/mm2_main/tests/docker_tests/tendermint_tests.rs Outdated Show resolved Hide resolved
0.008,
));
lazy_static! {
// Simple lock used for running the swap tests sequentially.
Copy link
Collaborator

Choose a reason for hiding this comment

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

but why tho?

Copy link
Member Author

Choose a reason for hiding this comment

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

They use same account seed; the same tendermint account tries to swap from multiple instances. Normally, we have this

pub(super) async fn seq_safe_send_raw_tx_bytes(
&self,
tx_payload: Any,
fee: Fee,
timeout_height: u64,
memo: String,
) -> Result<(String, Raw), TransactionErr> {
let (tx_id, tx_raw) = loop {
let tx_raw = try_tx_s!(self.any_to_signed_raw_tx(
try_tx_s!(self.priv_key_policy.activated_key_or_err()),
try_tx_s!(self.account_info(&self.account_id).await),
tx_payload.clone(),
fee.clone(),
timeout_height,
memo.clone(),
));
match self.send_raw_tx_bytes(&try_tx_s!(tx_raw.to_bytes())).compat().await {
Ok(tx_id) => break (tx_id, tx_raw),
Err(e) => {
if e.contains(ACCOUNT_SEQUENCE_ERR) {
debug!("Got wrong account sequence, trying again.");
continue;
}
return Err(crate::TransactionErr::Plain(ERRL!("{}", e)));
},
};
};
function which ensures we end up with the correct sequence id so the swap doesn't fail because of sequence mismatch errors, but somehow that didn't help (the node was keep giving the same sequence id instead of the new one, this is either docker testnet specific problem or it's cached for too long from the nucleus node).

Signed-off-by: onur-ozkan <work@onurozkan.dev>
shamardy
shamardy previously approved these changes Jul 2, 2024
Copy link
Collaborator

@shamardy shamardy left a comment

Choose a reason for hiding this comment

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

Great Work 🔥 ! Only one non-blocker comment.

btw, test_tendermint_ibc_withdraw and test_tendermint_ibc_withdraw_hd failed in the latest run so they seem a bit unstable, do you think this is acceptable @onur-ozkan?

@onur-ozkan
Copy link
Member Author

btw, test_tendermint_ibc_withdraw and test_tendermint_ibc_withdraw_hd failed in the latest run so they seem a bit unstable, do you think this is acceptable @onur-ozkan?

It seems like IBC relayer wasn't ready for connections when those tests were invoked. I will try to handle that properly. Thanks!

Signed-off-by: onur-ozkan <work@onurozkan.dev>
@onur-ozkan onur-ozkan force-pushed the offline-tendermint-testnets branch from 476c5aa to 6fd189e Compare July 3, 2024 14:35
Signed-off-by: onur-ozkan <work@onurozkan.dev>
Signed-off-by: onur-ozkan <work@onurozkan.dev>
Signed-off-by: onur-ozkan <work@onurozkan.dev>
@shamardy
Copy link
Collaborator

shamardy commented Jul 5, 2024

@mariocynicys @dimxy please check if your comments are resolved and if you have any more comments / review iterations.

Copy link
Collaborator

@mariocynicys mariocynicys left a comment

Choose a reason for hiding this comment

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

please check if your comments are resolved and if you have any more comments / review iterations.

Nope, LGTM

Copy link
Collaborator

@dimxy dimxy left a comment

Choose a reason for hiding this comment

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

LGTM

@shamardy shamardy merged commit 8e312a1 into dev Jul 5, 2024
18 of 25 checks passed
@shamardy shamardy deleted the offline-tendermint-testnets branch July 5, 2024 15:40
dimxy added a commit that referenced this pull request Jul 21, 2024
* dev:
  feat(nft-swap): add standalone maker contract and proxy support (#2100)
  feat(ETH): add `gas_limit` coins param to override default values (#2137)
  feat(tendermint): implement better sequence resolving logic (#2164)
  ci(artifact): add target for macos on apple silicon (#2163)
  fix(helpers): extend http to ws address conversion (#2166)
  fix(makerbot): add "testcoin" to provider options (#2161)
  fix(hd_wallet): make extended pubkey of hd wallet generic (#2159)
  fix(docker-tests): implement containers runtime directories (#2162)
  feat(tendermint): improve the `max` handling for tendermint withdraw (#2155)
  revert #2158 (comment) (#2160)
  ci(artifacts): upload build artifacts with in-tree script (#2158)
  test(tendermint): migrate to local/offline containerized testnets (#2128)
  use easingthemes/ssh-deploy@v5.0.3 for all builds except windows (#2157)
  chore(bin): rename mm2 binaries to kdf (#2126)
dimxy added a commit to dimxy/komodo-defi-framework that referenced this pull request Aug 12, 2024
* dev: (22 commits)
  chore(release): bump mm2 version to 2.2.0-beta (KomodoPlatform#2188)
  ci(docker-tests): ignore tendermint IBC tests for now (KomodoPlatform#2185)
  feat(nft-swap): complete refund methods (KomodoPlatform#2129)
  chore(release): add changelog entries for v2.1.0-beta (KomodoPlatform#2165)
  fix(zcoin): don't force low r signing to generate htlc pubkey for zcoin (KomodoPlatform#2184)
  chore(rust-analyzer): add rust-analyzer into the workspace toolchain (KomodoPlatform#2179)
  chore: migrate .cargo/config to .cargo/config.toml to avoid deprecation warning (KomodoPlatform#2177)
  fix(swaps): ensure taker payment spend confirmations (KomodoPlatform#2176)
  feat(nft-swap): add standalone maker contract and proxy support (KomodoPlatform#2100)
  feat(ETH): add `gas_limit` coins param to override default values (KomodoPlatform#2137)
  feat(tendermint): implement better sequence resolving logic (KomodoPlatform#2164)
  ci(artifact): add target for macos on apple silicon (KomodoPlatform#2163)
  fix(helpers): extend http to ws address conversion (KomodoPlatform#2166)
  fix(makerbot): add "testcoin" to provider options (KomodoPlatform#2161)
  fix(hd_wallet): make extended pubkey of hd wallet generic (KomodoPlatform#2159)
  fix(docker-tests): implement containers runtime directories (KomodoPlatform#2162)
  feat(tendermint): improve the `max` handling for tendermint withdraw (KomodoPlatform#2155)
  revert KomodoPlatform#2158 (comment) (KomodoPlatform#2160)
  ci(artifacts): upload build artifacts with in-tree script (KomodoPlatform#2158)
  test(tendermint): migrate to local/offline containerized testnets (KomodoPlatform#2128)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants