feat: upgraded security aspects in pre-commit hooks and added security scan gh workflow

[mpsc0x]

• extended pre-commit hooks for detecting sensitive data leaks
• added gitleaks and many other scanners to gh workflows
• upgraded compiler and cli to latest 7.7.0 version
• fixed coin migrator tests and logic acc. to new cli and compiler
• fixed npm audit

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.09%. Comparing base (ced9565) to head (f48fe15).
⚠️ Report is 11 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main      #47   +/-   ##
=======================================
  Coverage   97.09%   97.09%           
=======================================
  Files          16       16           
  Lines         516      516           
=======================================
  Hits          501      501           
  Misses         15       15           

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.

[matchv]

Does this invariant exist? I can have 100 APT legacy coin, but 0 APT FA as fa_balance, or I can receive 50 FA from another users, so at the end I have 100 APT coin, 50 APT FA.

Can you double check?

If I am correct, we just need to assert:

coin::balance<CoinType>(user) >= amount &&
amount > 0

[meng-xu-cs]

I also don't understand why this invariant is changed. As far as I understand, only APT is converted to the FA version (meaning, there is no legacy APT coin), but this does not generalize to all other coins, no?

If this indeed generalizes to all other coins, we should deprecate this coin_migrator module, as there will be no legacy coins to convert in the first place.

[mpsc0x]

@meng-xu-cs @matchv Here: https://github.com/aave/aptos-aave-v3/pull/47/files/95a4450d4d27c0fbeddeac7ca27bfb8d33d26362#diff-89a4dcffac1c03072f168d75352d0cdf43fff02474cdef01ebb2758fbdd04239R45

We are calling : https://github.com/aave/aptos-aave-v3/pull/47/files/95a4450d4d27c0fbeddeac7ca27bfb8d33d26362#diff-89a4dcffac1c03072f168d75352d0cdf43fff02474cdef01ebb2758fbdd04239R99

which as you see is calling coin::paired_metadata<CoinType>(); That is the mapping between coins and Fas. The entire coin_migrator right now is serving the purposes of being able to supply and map coins to FAs in the protocol. When you call coin_to_fa, the expectation is that there will be an already existing coin - fa mapping, otherwise it will fail. If there is a mapping (and that applies to APT and many other coins already), we will be dealing with atomically bonded units. Spending your FA decreases your coin in equal amounts and vice versa. The implicit conversion applies to APT only, indeed. All other coins need to be mapped the first time to an FA then they will become FAs. We cannot delete the coin_migrator upon an upgrade and we still need it in the supply_coin<CoinType> method in case one is to supply any coin different than APT.
@matchv is right that the check is a bit weird and it can directly be simplified to what he is suggesting. I have added this now.

[meng-xu-cs]

I also don't understand why this invariant is changed. As far as I understand, only APT is converted to the FA version (meaning, there is no legacy APT coin), but this does not generalize to all other coins, no?

If this indeed generalizes to all other coins, we should deprecate this coin_migrator module, as there will be no legacy coins to convert in the first place.

[matchv]

1st, the doc says: "Public entry functions", but here "entry" is absent, is it intended?

2nd, a spelling error on line 61, "CoinToFaConvertion" => "CoinToFaConversion"

3rd, While digging a bit into the Legacy Coin => FA migration, I also find the view function get_fa_balance() could do a write with line 99 "primary_fungible_store::ensure_primary_store_exists()", if it doesn't exist, it will create it, according to this official doc:

https://aptos.dev/move-reference/mainnet/aptos-framework/primary_fungible_store

It's worth considering alternative methods so we don't potentially write a record with a view function.

See this reference for the Legacy Coin <> FA balance check

https://aptos.dev/build/smart-contracts/fungible-asset#migration-from-coin-to-the-fungible-asset-standard

[mpsc0x]

I have addressed those comments @matchv Thanks.

[matchv]

lgtm