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

Enabled ttxt as a lib #22

Open
wants to merge 23 commits into
base: master
Choose a base branch
from

Conversation

iulianbarbu
Copy link
Collaborator

@iulianbarbu iulianbarbu commented Feb 6, 2025

Made the crate as a lib (in addition to being a binary) but..

Did a bunch on unnecessary changes too:

  • fixed clippy errors showing up in my editor (curious if you get any on your end, I am using 1.82)
  • enabled fmt to work on my IDE (nightly-2024-04-10)
  • upgraded packages (my crates.nvim extension showed me a few warnings with newer packages)
  • enabled taplo by adding a config file resembling the one in polkadot-sdk
  • one of the tests fails on my machine (oh_god2), so I ignored it.
  • upgrading the packages required at some point to drop the account_id() call on keyring, and use public_key() instead. I think this should be alright but lmk if you have another opinion.

I can revert the unnecessary changes if you want or find it important for some reason.

Closes #18

Signed-off-by: Iulian Barbu <iulian.barbu@parity.io>
...updated packages, made the crate a lib too

Signed-off-by: Iulian Barbu <iulian.barbu@parity.io>
@iulianbarbu iulianbarbu self-assigned this Feb 6, 2025
src/transaction.rs Outdated Show resolved Hide resolved
iulianbarbu and others added 5 commits February 11, 2025 17:01
…michalkucharczyk#20)

* fix: make derivation path compatible with pallet-balances devAccounts

Signed-off-by: Iulian Barbu <iulian.barbu@parity.io>

* remove SEED const

Signed-off-by: Iulian Barbu <iulian.barbu@parity.io>

---------

Signed-off-by: Iulian Barbu <iulian.barbu@parity.io>
Signed-off-by: Iulian Barbu <iulian.barbu@parity.io>
Signed-off-by: Iulian Barbu <iulian.barbu@parity.io>
Signed-off-by: Iulian Barbu <iulian.barbu@parity.io>
Signed-off-by: Iulian Barbu <iulian.barbu@parity.io>
@iulianbarbu iulianbarbu changed the title [WIP] Enabled ttxt as a lib Enabled ttxt as a lib Feb 11, 2025
Signed-off-by: Iulian Barbu <iulian.barbu@parity.io>
Signed-off-by: Iulian Barbu <iulian.barbu@parity.io>
Signed-off-by: Iulian Barbu <iulian.barbu@parity.io>
Signed-off-by: Iulian Barbu <iulian.barbu@parity.io>
Signed-off-by: Iulian Barbu <iulian.barbu@parity.io>
Signed-off-by: Iulian Barbu <iulian.barbu@parity.io>
src/main.rs Outdated Show resolved Hide resolved
src/scenario.rs Outdated Show resolved Hide resolved
src/scenario.rs Outdated Show resolved Hide resolved
iulianbarbu and others added 2 commits February 17, 2025 10:28
Signed-off-by: Iulian Barbu <iulian.barbu@parity.io>
src/main.rs Outdated Show resolved Hide resolved
src/main.rs Outdated Show resolved Hide resolved
src/scenario.rs Outdated Show resolved Hide resolved
src/scenario.rs Outdated Show resolved Hide resolved
pub mod scenario;
pub mod subxt_api_connector;
pub mod subxt_transaction;
pub mod transaction;
Copy link
Owner

Choose a reason for hiding this comment

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

One more clean-up that could be done in this PR is exposing only what is required in zombienet / main.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Zombienet tests will need the ScenarioBuilder, ScenarioExecutor, the DefaultTxTask and the transaction types: EthTransaction & SubstrateTransaction.

main.rs needs a few more.

There are a few more types that can be limited to pub(crate) or maybe remove pub(crate) altogether, but if done so Rust compiler complains that these types are related to other types that have higher visibility (meaning pub). In practice we can lower the visibility for all the types that are not directly used in main.rs & zombienet tests, and suppress the associated visibility warnings (which are a lot), but I decided to expose the types that rust compiler complains about, even if not directly used. I wouldn't overindex on the pub API that much, considering the way zombienet tests will be developed will rely on the types I mentioned. The rest of the types can extend ttxt for library users (e.g. add new tx builder, sink & txs), but at this time we're the only users.

I wouldn't publish ttxt on crates.io yet, but use it based on a git dependency in our tests for a bit of time until we decide more on how the API should look like (and after raising a bit more the tests coverage). WDYT?

iulianbarbu and others added 8 commits February 17, 2025 20:44
Signed-off-by: Iulian Barbu <iulian.barbu@parity.io>
Signed-off-by: Iulian Barbu <iulian.barbu@parity.io>
Signed-off-by: Iulian Barbu <iulian.barbu@parity.io>
Signed-off-by: Iulian Barbu <iulian.barbu@parity.io>
Co-authored-by: Michal Kucharczyk <1728078+michalkucharczyk@users.noreply.github.com>
Signed-off-by: Iulian Barbu <iulian.barbu@parity.io>
Co-authored-by: Michal Kucharczyk <1728078+michalkucharczyk@users.noreply.github.com>
Signed-off-by: Iulian Barbu <iulian.barbu@parity.io>
Signed-off-by: Iulian Barbu <iulian.barbu@parity.io>
Signed-off-by: Iulian Barbu <iulian.barbu@parity.io>
@iulianbarbu
Copy link
Collaborator Author

In latest batch of commits I added:

  • hash fix you gave me on Element
  • tip cli flag patch you sent me on Element
  • added derive_accounts test in subxt_transaction
  • lib usage example

.with_send_threshold(*send_threshold as usize)
.with_block_monitoring(*block_monitor)
.with_watched_txs(!unwatched)
.with_tip(*tip);
Copy link
Owner

@michalkucharczyk michalkucharczyk Feb 18, 2025

Choose a reason for hiding this comment

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

tip could be overwritten by lines 74-75 (remark recipe setting).
Maybe tip should be a dedicated field in ScenarioBuilder - otherwise it could be overwritten when recipe is set after. (or should be moved from old recipe when new is set).

.with_send_threshold(*send_threshold as usize)
.with_block_monitoring(*block_monitor)
.with_watched_txs(!unwatched)
.with_tip(*tip);
Copy link
Owner

Choose a reason for hiding this comment

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

same here

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.

Split code to bin and lib
2 participants