Skip to content

feat: Added programmable_transaction.move and related test #8146

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
Merged
merged 8 commits into from
Sep 2, 2025
Merged

feat: Added programmable_transaction.move and related test #8146

merged 8 commits into from
Sep 2, 2025

Conversation

@theiari
Copy link
Contributor

@theiari theiari commented Aug 8, 2025 •
edited by miker83z
Loading

Description of change

Introduced programmable_transaction.move and auth_context.move in the framework, in order to represent these types in Move.

Closes #8122
Closes #8165

How the change has been tested

programmable_transaction_tests has been tested using iota move test

Release Notes

@theiari theiari linked an issue Aug 8, 2025 that may be closed by this pull request
[AA][auth] PTB Type Move representation #8122
Closed
@vercel
Copy link

vercel bot commented Aug 8, 2025 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for GitHub.

6 Skipped Deployments
Project Deployment Preview Comments Updated (UTC)
apps-backend Ignored Ignored Preview Sep 2, 2025 7:52am
apps-ui-kit Ignored Ignored Preview Sep 2, 2025 7:52am
iota-evm-bridge Ignored Ignored Preview Sep 2, 2025 7:52am
iota-multisig-toolkit Ignored Ignored Preview Sep 2, 2025 7:52am
rebased-explorer Ignored Ignored Preview Sep 2, 2025 7:52am
wallet-dashboard Ignored Ignored Preview Sep 2, 2025 7:52am

miker83z
miker83z reviewed Aug 8, 2025
View reviewed changes
@miker83z
Copy link
Contributor

miker83z commented Aug 8, 2025

No need to represent VectorType, Box and StructInput as well

@miker83z miker83z added the vm-language Issues related to the VM & Language Team label Aug 8, 2025
@theiari
fix: poolished auth_context.move
b1844da 
feat: added simple getter test
miker83z
miker83z requested changes Aug 12, 2025
View reviewed changes
@theiari
refactor: remove auth_context module and related tests, migrate to pr…
1dcdfb4 
…ogrammable_transaction

fix: updated programmable_transaction_tests.move to pass both types and args for the ptb move call
miker83z
miker83z requested changes Aug 13, 2025
View reviewed changes
@theiari
fix: minor changes to programmable_transaction_tests.move
f21cdcc 
refactor: changed structure of programmable_transaction.move for better clarity. removed get_ to getters
feat: added new test-only constructor for a programmable transaction
@theiari theiari changed the title feat: added auth_context.move module Added programmable_transaction.move and related test Aug 18, 2025
@theiari theiari changed the title Added programmable_transaction.move and related test feat: Added programmable_transaction.move and related test Aug 18, 2025
@theiari
feat: added auth_context.move
99d00ea 
feat: added auth_context_tests.move
miker83z
miker83z approved these changes Aug 19, 2025
View reviewed changes
@miker83z miker83z changed the base branch from develop to vm-lang/aa-auth/8116-feature-branch August 19, 2025 09:31
@miker83z miker83z linked an issue Aug 19, 2025 that may be closed by this pull request
[AA][auth] Represent the AuthContext in move #8165
Closed
@miker83z miker83z marked this pull request as ready for review August 19, 2025 09:34
@miker83z miker83z requested a review from a team as a code owner August 19, 2025 09:34
@TheMrAI TheMrAI requested review from Dkwcs, TheMrAI and valeriyr August 22, 2025 07:36
valeriyr
valeriyr requested changes Aug 22, 2025
View reviewed changes
Copy link
Contributor

@valeriyr valeriyr left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Looks good!
Please, check the following tests:
https://github.com/iotaledger/iota/actions/runs/17121012254/job/48562298279?pr=8146
You need to fix at least this one: build_system_packages.

I have a question as well. We need to update the framework snapshots after changes like this. Are we going to do this in the current PR or later?

crates/iota-framework/packages/iota-framework/tests/programmable_transaction_tests.move
new_programmable_transaction
};
use std::type_name;

Copy link
Contributor

@valeriyr valeriyr Aug 22, 2025

Choose a reason for hiding this comment

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

I think it would be nice to add the tests that check all the errors we can have, to "freeze" this behavior in the tests.
What do you think?

@TheMrAI
Copy link
Contributor

TheMrAI commented Aug 22, 2025

Looks good! Please, check the following tests: https://github.com/iotaledger/iota/actions/runs/17121012254/job/48562298279?pr=8146 You need to fix at least this one: build_system_packages.

I have a question as well. We need to update the framework snapshots after changes like this. Are we going to do this in the current PR or later?

I think we should update it here. Just to try and keep the history saner, in case we want to decide later how to merge the whole shebang.

TheMrAI
TheMrAI suggested changes Aug 22, 2025
View reviewed changes
Copy link
Contributor

@TheMrAI TheMrAI left a comment

Choose a reason for hiding this comment

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

I would seriously consider removing all functions that do:
"match (arg) {
type => internal,
_ => Abort
}"

These functions are bloating the interface and provide little functionality. The caller of the library should be able to decide if they want to fail if something doesn't match, if they want to test it etc.
They have all the tools at their disposal as none of the enums used are private.

On a similar notion I would remove "is_pure_data", "is_object_data" stile functions as well. Let the user decide what they want to test and how. For example if they wanted to test if it is "ImmOrOwnedObject" they have to write the test themselves. There is no need to provide these.

If you do this, you can ignore my other comments regarding outputs as well.
The interface becomes much cleaner, you need to test much less.

@theiari
fix: updated published_api with new move modules
4a2328c 
fix: set all getters to return referenced variables
fix: added few extra comments into programmable_transaction structs
refactor: renamed command_variant_name to command_to_int for better understanding

test: updated programmable_transaction_tests for type safety and abort codes check
@miker83z miker83z merged commit 0dfba5a into vm-lang/aa-auth/8116-feature-branch Sep 2, 2025
33 of 36 checks passed
@miker83z miker83z deleted the vm-lang/aa-auth/8122-ptb-type-in-move branch September 2, 2025 08:00
@TheMrAI TheMrAI mentioned this pull request Sep 4, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@miker83z miker83z miker83z approved these changes

@valeriyr valeriyr valeriyr approved these changes

@Dkwcs Dkwcs Awaiting requested review from Dkwcs

+1 more reviewer

@TheMrAI TheMrAI TheMrAI approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

vm-language Issues related to the VM & Language Team

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[AA][auth] Represent the AuthContext in move [AA][auth] PTB Type Move representation

5 participants

@theiari @miker83z @TheMrAI @valeriyr