Skip to content

Test Mint and Burn - #1010

Open
Primata wants to merge 29 commits into
mainfrom
test-lock-mint-client
Open

Test Mint and Burn#1010
Primata wants to merge 29 commits into
mainfrom
test-lock-mint-client

Conversation

@Primata

@Primata Primata commented Jan 20, 2025

Copy link
Copy Markdown
Contributor

Summary

  • RFCs: Link to RFC, Link to RFC, or $\emptyset$.
  • Categories: any of protocol-units, networks, scripts, util, cicd, or misc.

#1009

Burn elsa's supply.

Changelog

moved away from native_bridge integration to e2e testing

added burn_from and mint_to methods gated to framework signer

Testing

networks/movement/movement-client/src/bin/e2e/lock_mint.rs

rm -rf .movement/ && RUST_LOG=info CELESTIA_LOG_LEVEL=FATAL CARGO_PROFILE=release CARGO_PROFILE_FLAGS=--release nix develop --extra-experimental-features nix-command --extra-experimental-features flakes --command bash -c "just movement-full-node native build.setup.eth-local.celestia-local.test-mint-burn --keep-tui"

Outstanding issues

We might need to add a burn_from entry function on the core_resources_account.

@Primata
Primata requested a review from ganymedio as a code owner January 21, 2025 00:19
@0xmovses

Copy link
Copy Markdown
Contributor

We all try to stick to this commit message standard (roughly)
https://www.conventionalcommits.org/en/v1.0.0/

@ganymedio ganymedio left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

When I run

RUST_LOG=info CELESTIA_LOG_LEVEL=FATAL CARGO_PROFILE=release CARGO_PROFILE_FLAGS=--release nix develop --extra-experimental-features nix-command --extra-experimental-features flakes --command bash -c "just movement-full-node native build.setup.eth-local.celestia-local.test-lock-mint --keep-tui"

I get

Error: Failed to create dead account

caused by

API error Error(VmError): Invalid transaction: Type: Validation Code: SEQUENCE_NUMBER_TOO_OLD

image

@Primata

Primata commented Jan 25, 2025

Copy link
Copy Markdown
Contributor Author

When I run


RUST_LOG=info CELESTIA_LOG_LEVEL=FATAL CARGO_PROFILE=release CARGO_PROFILE_FLAGS=--release nix develop --extra-experimental-features nix-command --extra-experimental-features flakes --command bash -c "just movement-full-node native build.setup.eth-local.celestia-local.test-lock-mint --keep-tui"

I get

Error: Failed to create dead account

caused by

API error Error(VmError): Invalid transaction: Type: Validation Code: SEQUENCE_NUMBER_TOO_OLD

image

@andygolay please clear .movement/ before every run.

Updated command to do it before hand

@0xmovses

Copy link
Copy Markdown
Contributor

As far I can see there is no NativeBridge.sol deployment on anvil here, and MOVE deposit call. @andygolay not ready for review yet.

@0xmovses
0xmovses marked this pull request as draft January 25, 2025 15:52
@Primata
Primata requested a review from ganymedio January 26, 2025 03:17
@0xmovses 0xmovses changed the title Test Lock and Mint Test Mint and Burn Jan 26, 2025
@Primata
Primata marked this pull request as ready for review January 27, 2025 02:57

@0xmovses 0xmovses left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for this. Remove all println!s for tracing::info. Cleanup imports, use tree style imports. The comments you've written right at the bottom are correct but lets remove the comment from the SC. Rename the test to mint_burn.rs and any associated process_compose naming and cmd / pkgs.

Thanks!

@0xmovses

Copy link
Copy Markdown
Contributor

Merge conflicts will auto-resolve after:

  1. Governed Gas Pool aptos-core#114 (review) is approved and merged
  2. PR updates movement/main aptos deps to use branch = movement

@Primata
Primata requested a review from 0xmovses January 27, 2025 04:19

@ganymedio ganymedio left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@ganymedio ganymedio left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Added one change suggestion, to assert dead address balance is 0 at the end.

Comment thread networks/movement/movement-client/src/bin/e2e/mint_burn.rs
@0xmovses

Copy link
Copy Markdown
Contributor

Even when I remove .movement/ here I get the error

Error: Failed to create dead account

Caused by:
    API error Error(VmError): Invalid transaction: Type: Validation Code: SEQUENCE_NUMBER_TOO_OLD

Wrong increment?

@0xmovses

0xmovses commented Feb 4, 2025

Copy link
Copy Markdown
Contributor

Is there a new dependent PR in aptos-core that contains the mint_to function? O

@Primata

Primata commented Feb 4, 2025

Copy link
Copy Markdown
Contributor Author

Is there a new dependent PR in aptos-core that contains the mint_to function? O

Same branch and PR add_burn_from

@0xmovses

0xmovses commented Feb 4, 2025

Copy link
Copy Markdown
Contributor

PR number or link?

@0xmovses

0xmovses commented Feb 6, 2025

Copy link
Copy Markdown
Contributor

Ok this is ready to go in can you fix the merge conflicts and add this test to the Checks.yml all new e2e test should be added to CI.

@l-monninger

Copy link
Copy Markdown
Contributor

Will pull again.

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.

4 participants