pendulum-chain / spacewalk

Apache License 2.0
34 stars 7 forks source link

482 add stellar testnet archive urls to vault client config #524

Closed gianfra-t closed 3 months ago

gianfra-t commented 4 months ago

Regarding #482

About

Adds archive url's for Stellar testent, and code changes such that we don't discriminate anymore between testnet and mainnet for fetching this values.

Notable changes

gianfra-t commented 4 months ago

@pendulum-chain/devs beyond the simple unit tests added, does anyone know if there is a configuration by which we can trigger in the integration tests a situation in which the client will try to look for these archives?

b-yap commented 4 months ago

@gianfra-t Waiting time is 5 minutes:

During normal operation, a validating stellar-core server will save a "checkpoint" of its recent operations to XDR files, compress them, and publish them to all of its configured writable history archives once every 64 ledgers (about once every 5 minutes).

There is an integration test that Marcel added: https://github.com/pendulum-chain/spacewalk/blob/1c3ea1c163e721267e45a8f8baae3b4964df6ff3/clients/vault/tests/vault_integration_tests.rs#L609-L703

You can see in line 663: https://github.com/pendulum-chain/spacewalk/blob/1c3ea1c163e721267e45a8f8baae3b4964df6ff3/clients/vault/tests/vault_integration_tests.rs#L663




@ebma if the archives are already available in testnet, should we revert the:

Screenshot 2024-05-09 at 5 09 46 PM

testchain to only have 1? It could reduce the code.

gianfra-t commented 4 months ago

Thanks for pointing that out @b-yap ! I replaced here such that the testnet would be used and it works (locally).

I didn't add a new test_issue_execution_succeeds_from_archive just for testnet since I think it may be redundant, but we could.

b-yap commented 4 months ago

hhhm, something wrong with the rustfmt; it's reverted back. I tried to do cargo +nightly-2024-02-09 and it accepts ; after returns.

gianfra-t commented 4 months ago

I did run fmt because the CI failed on the format check, do you think this broke something?

b-yap commented 3 months ago

trailing_semicolon is an unstable option, and so are the others. Probably makes sense why it works only now and then.

Torsten commented on this before. I didn't want the reformatting to overlap with this PR.

Apologies on the unverified commits. :sob: @gianfra-t if you can, please sign the commits again :bow:.

gianfra-t commented 3 months ago

Oh okay, sorry! So we have to be careful and leave the format to the pre-commit hook @b-yap? Signing the commits again no problem.

ebma commented 3 months ago

@gianfra-t and @b-yap I created a new issue https://github.com/pendulum-chain/spacewalk/issues/525 for the formatting. I already noticed this problem last week and it's quite annoying that we now have this mismatch between the formatting of the precommit hook vs what the CI expects.

if the archives are already available in testnet, should we revert the testchain to only have 1? It could reduce the code.

@b-yap I don't think we should. The main reason I created the tests is to be able to replicate issues that only seem to occur on Stellar mainnet. Being able to test on both Stellar test and main network is quite valuable IMO.

I didn't add a new test_issue_execution_succeeds_from_archive just for testnet since I think it may be redundant, but we could.

True, it's redundant but I'd say we add the test case anyway. We can afford to have the CI run 3 extra minutes in order to be safe that restoring messages from archives works as expected. In the future it could happen that the SDF introduces some changes that break our logic and as they will introduce them on testnet first, we would be able to catch it earlier.