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.

Sign up for GitHub

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

Jump to bottom

generating test transactions with move script #775

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

generating test transactions with move script #775

wants to merge 2 commits into from

Conversation

yuunlimm
Copy link
Contributor

@yuunlimm yuunlimm commented Jan 17, 2025 •
edited
Loading

Description

Screenshot 2025-01-17 at 10.50.19 AM.png

Checklist

  • If any existing pages were renamed or removed:
    • Were redirects added to next.config.mjs?
    • Did you update any relative links that pointed to the renamed / removed pages?
  • Do all Lints pass?
    • Have you ran pnpm fmt?
    • Have you ran pnpm lint?

@vercel Vercel
Copy link

vercel bot commented Jan 17, 2025 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
developer-docs-nextra ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 24, 2025 7:46pm

@yuunlimm yuunlimm mentioned this pull request Jan 17, 2025
Merged
2 tasks
@yuunlimm Graphite App
Copy link
Contributor Author

yuunlimm commented Jan 17, 2025 •
edited
Loading

This stack of pull requests is managed by Graphite. Learn more about stacking.

@yuunlimm yuunlimm force-pushed the 01-17-generating_test_transactions_with_move_script branch from f346475 to 337099f Compare January 17, 2025 18:50
@yuunlimm yuunlimm marked this pull request as ready for review January 17, 2025 18:50
@yuunlimm yuunlimm requested review from a team, gregnazario, hariria and igor-p as code owners January 17, 2025 18:50
This was referenced Jan 17, 2025
Draft
Open
@yuunlimm yuunlimm force-pushed the 01-17-generating_test_transactions_with_move_script branch from 337099f to 41c221f Compare January 23, 2025 18:16
@yuunlimm yuunlimm force-pushed the 01-17-importing_test_transactions branch from d35779c to 18f5a07 Compare January 23, 2025 18:22
@yuunlimm yuunlimm force-pushed the 01-17-generating_test_transactions_with_move_script branch from 41c221f to f4a5496 Compare January 23, 2025 18:22
@yuunlimm yuunlimm force-pushed the 01-17-importing_test_transactions branch from 18f5a07 to e3a6dae Compare January 23, 2025 18:26
@yuunlimm yuunlimm force-pushed the 01-17-generating_test_transactions_with_move_script branch from f4a5496 to 7c9ef01 Compare January 23, 2025 18:26
@yuunlimm yuunlimm force-pushed the 01-17-importing_test_transactions branch from e3a6dae to 51b360e Compare January 23, 2025 18:27
@yuunlimm yuunlimm force-pushed the 01-17-generating_test_transactions_with_move_script branch from 7c9ef01 to 443f3c1 Compare January 23, 2025 18:27
@yuunlimm yuunlimm force-pushed the 01-17-importing_test_transactions branch from 51b360e to 0307421 Compare January 23, 2025 18:30
@yuunlimm yuunlimm force-pushed the 01-17-generating_test_transactions_with_move_script branch from 443f3c1 to 719d843 Compare January 23, 2025 18:30
@yuunlimm yuunlimm force-pushed the 01-17-importing_test_transactions branch from 0307421 to d1e31ef Compare January 23, 2025 18:33
@yuunlimm yuunlimm force-pushed the 01-17-generating_test_transactions_with_move_script branch from 719d843 to fc57bc1 Compare January 23, 2025 18:34
rtso
rtso reviewed Jan 23, 2025
View reviewed changes
@yuunlimm yuunlimm force-pushed the 01-17-generating_test_transactions_with_move_script branch from fc57bc1 to ee03a7e Compare January 23, 2025 19:47
graphite-app[bot]
graphite-app bot reviewed Jan 23, 2025
View reviewed changes
apps/nextra/pages/en/build/indexer/indexer-sdk/documentation/advanced-tutorials/txn-script.mdx Outdated


## Next Steps
Once the transaction constants are integrated, you can use them in processor tests to validate functionality. For detailed instructions on writing processor tests, refer to [Writing Processor Tests]().
Copy link
Contributor

Choose a reason for hiding this comment

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

The empty link [Writing Processor Tests]() will be broken when rendered. Please either add the correct URL path to the parentheses or remove the Markdown link formatting if the destination page doesn't exist yet.

Spotted by Graphite Reviewer

Is this helpful? React 👍 or 👎 to let us know.

apps/nextra/pages/en/build/indexer/indexer-sdk/documentation/advanced-tutorials/txn-script.mdx Outdated

1. Read the configuration in the `move_fixtures` folder.
2. Execute the specified Move scripts.
3. Output the generated JSON files to the designated folder (`~/aptos-core/ecosystem/indexer-grpc/indexer-transaction-generator/json_transactions`).
Copy link
Contributor

Choose a reason for hiding this comment

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

The output path shown in the documentation (~/aptos-core/ecosystem/indexer-grpc/indexer-transaction-generator/json_transactions) differs from the path specified in the command (../indexer-test-transactions/src/). This discrepancy should be resolved to ensure users can follow the instructions correctly. Consider updating either the command or the documentation to show the correct output path.

Spotted by Graphite Reviewer

Is this helpful? React 👍 or 👎 to let us know.

@yuunlimm yuunlimm force-pushed the 01-17-importing_test_transactions branch from 5fd548e to aeacbb0 Compare January 23, 2025 21:16
@yuunlimm yuunlimm force-pushed the 01-17-generating_test_transactions_with_move_script branch from ee03a7e to 8736bb3 Compare January 23, 2025 21:17
@yuunlimm yuunlimm force-pushed the 01-17-importing_test_transactions branch from aeacbb0 to 60db79c Compare January 23, 2025 21:38
@yuunlimm yuunlimm force-pushed the 01-17-generating_test_transactions_with_move_script branch from 8736bb3 to 260f684 Compare January 23, 2025 21:38
rtso
rtso reviewed Jan 27, 2025
View reviewed changes
apps/nextra/pages/en/build/indexer/indexer-sdk/documentation/advanced-tutorials/txn-script.mdx Outdated
### Set up `move_fixtures` folder
Before proceeding, ensure you have the move_fixtures folder set up in the appropriate location:
1. Location:
The move_fixtures folder should be created in the `aptos-core/ecosystem/indexer-grpc/indexer-transaction-generator/imported_transactions` directory. This is the folder where Move scripts and their configurations for test transactions will be stored.
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
The move_fixtures folder should be created in the `aptos-core/ecosystem/indexer-grpc/indexer-transaction-generator/imported_transactions` directory. This is the folder where Move scripts and their configurations for test transactions will be stored.
The `move_fixtures` folder should be created in the `aptos-core/ecosystem/indexer-grpc/indexer-transaction-generator/imported_transactions` directory. This is the folder where Move scripts and their configurations for test transactions will be stored.

apps/nextra/pages/en/build/indexer/indexer-sdk/documentation/advanced-tutorials/txn-script.mdx Outdated
1. Location:
The move_fixtures folder should be created in the `aptos-core/ecosystem/indexer-grpc/indexer-transaction-generator/imported_transactions` directory. This is the folder where Move scripts and their configurations for test transactions will be stored.

<Callout> **Note:** Do not create the `move_fixtures` folder in your processor repository. All Move-related files should reside in the `aptos-core` repository under the specified directory. </Callout>
Copy link
Contributor

Choose a reason for hiding this comment

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

why can't you put move_fixtures in the processor repository? what if they want to add the fixtures to version control?

rtso
rtso reviewed Jan 27, 2025
View reviewed changes
apps/nextra/pages/en/build/indexer/indexer-sdk/documentation/advanced-tutorials/txn-script.mdx Outdated
### Set Up Test Accounts

1. These accounts will be used to deploy your module.
2. Set up as many accounts as you need. These accounts will be used to send the scripted transactions. Refer to the guide [here](https://aptos.dev/en/build/cli/setup-cli) to create accounts.
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
2. Set up as many accounts as you need. These accounts will be used to send the scripted transactions. Refer to the guide [here](https://aptos.dev/en/build/cli/setup-cli) to create accounts.
2. Set up as many accounts as you need. These accounts will be used to send the scripted transactions. Refer to [Aptos CLI](https://aptos.dev/en/build/cli/setup-cli) to create accounts.

apps/nextra/pages/en/build/indexer/indexer-sdk/documentation/advanced-tutorials/txn-script.mdx Outdated

## Overview:

This section outlines how to create test transactions with Move scripts.
Copy link
Contributor

Choose a reason for hiding this comment

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

i think it'd help to have details on why you need scripted txn's, and when you would use a scripted txn instead of an imported txn

apps/nextra/pages/en/build/indexer/indexer-sdk/documentation/advanced-tutorials/txn-script.mdx Outdated
Comment on lines 90 to 92
- **Add as a Dependency**:

Include the crate containing the generated transactions as a dependency in the `Cargo.toml` file of your test crate. (Internally, transactions are stored in `aptos-core` and used in the [processor repo](https://github.com/aptos-labs/aptos-indexer-processors/blob/0c92d323b0f560b5f8601a831a36520ad9b72d68/rust/Cargo.toml#L34)).
Copy link
Contributor

Choose a reason for hiding this comment

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

not needed as per discussion on previous PR

apps/nextra/pages/en/build/indexer/indexer-sdk/documentation/advanced-tutorials/txn-script.mdx Outdated

```bash
cd ~/aptos-core/ecosystem/indexer-grpc/indexer-transaction-generator
cargo run -- --testing-folder ./imported_transactions --output-folder ../indexer-test-transactions/src/
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
cargo run -- --testing-folder ./imported_transactions --output-folder ../indexer-test-transactions/src/
cargo run -- --testing-folder ./imported_transactions --output-folder /path/to/processor/repo/

@yuunlimm yuunlimm force-pushed the 01-17-importing_test_transactions branch from 60db79c to e8f5eaf Compare February 8, 2025 00:59
@yuunlimm yuunlimm force-pushed the 01-17-generating_test_transactions_with_move_script branch from 260f684 to 95c1024 Compare February 8, 2025 00:59
@yuunlimm yuunlimm force-pushed the 01-17-importing_test_transactions branch from e8f5eaf to 775613a Compare February 8, 2025 01:03
@yuunlimm yuunlimm force-pushed the 01-17-generating_test_transactions_with_move_script branch from 95c1024 to d958d58 Compare February 8, 2025 01:03
graphite-app[bot]
graphite-app bot reviewed Feb 8, 2025
View reviewed changes
apps/nextra/pages/en/build/indexer/indexer-sdk/advanced-tutorials/txn-script.mdx Outdated
The move_fixtures folder should be created in the `aptos-core/ecosystem/indexer-grpc/indexer-transaction-generator/imported_transactions` directory. This is the folder where Move scripts and their configurations for test transactions will be stored.

<Callout> **Note:** Do not create the `move_fixtures` folder in your processor repository. All Move-related files should reside in the `aptos-core` repository under the specified directory. </Callout>
2. Steps to set up the follder:
Copy link
Contributor

Choose a reason for hiding this comment

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

Typo: follder -> folder

Spotted by Graphite Reviewer

Is this helpful? React 👍 or 👎 to let us know.

apps/nextra/pages/en/build/indexer/indexer-sdk/advanced-tutorials/txn-script.mdx Outdated


## Next Steps
Once the transaction constants are integrated, you can use them in processor tests to validate functionality. For detailed instructions on writing processor tests, refer to [Writing Processor Tests]().
Copy link
Contributor

Choose a reason for hiding this comment

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

The link [Writing Processor Tests]() appears to be a placeholder without a destination URL. Consider either adding the correct documentation URL or removing the link formatting until the documentation is available. This will prevent broken links in the published documentation.

Spotted by Graphite Reviewer

Is this helpful? React 👍 or 👎 to let us know.

@yuunlimm yuunlimm changed the base branch from 01-17-importing_test_transactions to graphite-base/775 February 8, 2025 01:29
@yuunlimm yuunlimm force-pushed the 01-17-generating_test_transactions_with_move_script branch from d958d58 to 72c37ec Compare February 8, 2025 01:29
@yuunlimm yuunlimm changed the base branch from graphite-base/775 to main February 8, 2025 01:30
@yuunlimm yuunlimm force-pushed the 01-17-generating_test_transactions_with_move_script branch from 72c37ec to 8313ef5 Compare February 8, 2025 01:30
hariria
hariria reviewed Feb 8, 2025
View reviewed changes
apps/nextra/pages/en/build/indexer/indexer-sdk/advanced-tutorials/txn-script.mdx Outdated
@@ -0,0 +1,102 @@
---
title: "Aptos Transaction generated using Move Script"
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it might be good to make the title of this consistent with the sidebar title you have in _meta.tsx

@yuunlimm yuunlimm force-pushed the 01-17-generating_test_transactions_with_move_script branch from 8313ef5 to 9d90f22 Compare February 12, 2025 18:31
graphite-app[bot]
graphite-app bot reviewed Feb 12, 2025
View reviewed changes
apps/nextra/pages/en/build/indexer/indexer-sdk/advanced-tutorials/txn-script.mdx
The configuration file should be structured like this:
- output_name: This field specifies the name of the output file where the results of the transaction will be saved.
- script_path: This field holds the path to the Move script file that will be executed as part of the transaction.
- sender_address: : This field contains the address of the account that will send the transaction.
Copy link
Contributor

Choose a reason for hiding this comment

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

There is a duplicate colon in sender_address: : - one colon is sufficient here.

Spotted by Graphite Reviewer

Is this helpful? React 👍 or 👎 to let us know.

apps/nextra/pages/en/build/indexer/indexer-sdk/advanced-tutorials/txn-script.mdx

### Export the `json_transactions` Folder

Since the `generated_transactions.rs` relies on the `json_transactions` Ensure the `json_transactions` folder is properly exported in the library file for your tests have direct access to the transaction data.
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing comma after "json_transactions" in this sentence. The corrected version should read: "Since the generated_transactions.rs relies on the json_transactions, ensure..."

Spotted by Graphite Reviewer

Is this helpful? React 👍 or 👎 to let us know.

@yuunlimm yuunlimm requested a review from rtso February 12, 2025 22:25
graphite-app[bot]
graphite-app bot reviewed Feb 24, 2025
View reviewed changes
apps/nextra/pages/en/build/indexer/indexer-sdk/advanced-tutorials/txn-script.mdx
The configuration file should be structured like this:
- output_name: This field specifies the name of the output file where the results of the transaction will be saved.
- script_path: This field holds the path to the Move script file that will be executed as part of the transaction.
- sender_address: : This field contains the address of the account that will send the transaction.
Copy link
Contributor

Choose a reason for hiding this comment

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

There is a duplicate colon in sender_address: : - this should be sender_address:

Spotted by Graphite Reviewer

Is this helpful? React 👍 or 👎 to let us know.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gregnazario gregnazario

@hariria hariria

@graphite-app graphite-app[bot]

@igor-p igor-p

@rtso rtso

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@yuunlimm @gregnazario @rtso @hariria