Skip to content

Conversation

@mpsc0x
Copy link
Collaborator

@mpsc0x mpsc0x commented Aug 21, 2025

No description provided.

@mpsc0x mpsc0x self-assigned this Aug 21, 2025
@mpsc0x mpsc0x force-pushed the feat/mpsc0x/aave-scripts branch from 1a8d7d3 to 070f65c Compare August 21, 2025 08:48
@mpsc0x mpsc0x force-pushed the feat/mpsc0x/aave-scripts branch from 070f65c to 96bf9ba Compare August 21, 2025 08:54
@mpsc0x mpsc0x force-pushed the feat/mpsc0x/aave-scripts branch from 96bf9ba to da0543e Compare August 21, 2025 08:58
@codecov
Copy link

codecov bot commented Aug 21, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (main@64c9c1d). Learn more about missing BASE report.
⚠️ Report is 15 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main      #51   +/-   ##
=======================================
  Coverage        ?   97.09%           
=======================================
  Files           ?       16           
  Lines           ?      516           
  Branches        ?        0           
=======================================
  Hits            ?      501           
  Misses          ?       15           
  Partials        ?        0           
Flag Coverage Δ
move 97.09% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

harshbhatt18
harshbhatt18 previously approved these changes Aug 21, 2025
Copy link
Collaborator

@meng-xu-cs meng-xu-cs left a comment

Choose a reason for hiding this comment

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

Since this Move script is purely demonstrative purpose and is not part of the Aave protocol, I wouldn't recommend putting it under the aave-core directory, nor have it linked up with the Makefile.

Instead, if possible, we can have an examples or tutorial directory and put the flashloan scripts as a Move package there, i.e.,

<git-repo-directory>/examples/flashloan where in this directory we have Move.toml which already hardcode the addresses of relevant Aave packages in the .toml file (instead of passing them via the Makefile).

Users should be able to invoke the Move scripts via aptos move run-script command under that examples/flashloan directory.

@matchv
Copy link
Collaborator

matchv commented Aug 22, 2025

Agree with @meng-xu-cs that we better create a separate folder for this script.

@mpsc0x mpsc0x force-pushed the feat/mpsc0x/aave-scripts branch from 7ee3e1e to dd63aab Compare August 22, 2025 08:08
@mpsc0x mpsc0x force-pushed the feat/mpsc0x/aave-scripts branch from dd63aab to 3fc8f4c Compare August 22, 2025 08:12
@mpsc0x mpsc0x force-pushed the feat/mpsc0x/aave-scripts branch from 2f02c78 to 3250600 Compare August 22, 2025 08:16
@mpsc0x
Copy link
Collaborator Author

mpsc0x commented Aug 22, 2025

Since this Move script is purely demonstrative purpose and is not part of the Aave protocol, I wouldn't recommend putting it under the aave-core directory, nor have it linked up with the Makefile.

Instead, if possible, we can have an examples or tutorial directory and put the flashloan scripts as a Move package there, i.e.,

<git-repo-directory>/examples/flashloan where in this directory we have Move.toml which already hardcode the addresses of relevant Aave packages in the .toml file (instead of passing them via the Makefile).

Users should be able to invoke the Move scripts via aptos move run-script command under that examples/flashloan directory.

I followed your advice and have moved the package into examples now. There is a separate Makefile inside examples as the commands are quite complex and I wanted the users to be guided all the way. Also, I have decided not to hardcore the values as we can elegantly pass them through the Makefile command without polluting the example. I find it more elegant and it naturally follows the root Makefile idea and structure. I have also added a pipeline compile check for them and updated the README. cc @matchv @meng-xu-cs

…in permissions

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
@mpsc0x mpsc0x force-pushed the feat/mpsc0x/aave-scripts branch from 3250600 to 78fadd3 Compare August 22, 2025 08:28
@mpsc0x mpsc0x changed the title feat: Added aave-scripts with commands and flashloan calls feat: Added examples package with commands and flashloan examples Aug 22, 2025
Copy link
Collaborator

@matchv matchv left a comment

Choose a reason for hiding this comment

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

lgtm

@mpsc0x mpsc0x merged commit fef93f5 into main Aug 25, 2025
19 checks passed
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.

5 participants