feat: added gho direct minter #57

mpsc0x · 2025-09-02T06:21:46Z

No description provided.

codecov · 2025-09-04T10:51:05Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.55%. Comparing base (822a67e) to head (5ec5926).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #57      +/-   ##
==========================================
- Coverage   97.09%   96.55%   -0.54%     
==========================================
  Files          16       16              
  Lines         516      523       +7     
==========================================
+ Hits          501      505       +4     
- Misses         15       18       +3

Flag	Coverage Δ
move	`96.55% <100.00%> (-0.54%)`	⬇️

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.

fix: add token to git config for actions workflow

matchv · 2025-09-11T04:39:28Z

aave-core/sources/gho-direct-minter/gho_direct_minter.move

+        let gho_reserve_entity = signer::address_of(&minter_signer);
+
+        // check: gho_reserve_entity must have RISK ADMIN ROLE for the pool
+        is_risk_admin(gho_reserve_entity);


[Critical]
is_risk_admin() returns a bool
we need an assert here:

assert!(is_risk_admin(gho_reserve_entity), error_config::get_ecaller_not_risk_or_pool_admin())

matchv · 2025-09-11T04:40:34Z

aave-core/sources/gho-direct-minter/gho_direct_minter.move

+        let gho_reserve_entity = signer::address_of(&minter_signer);
+
+        // check: gho_reserve_entity must have RISK ADMIN ROLE for the pool
+        is_risk_admin(gho_reserve_entity);


Need an assert to enforce "is_risk_admin()"
assert!(is_risk_admin(gho_reserve_entity), error_config::get_ecaller_not_risk_or_pool_admin());

matchv · 2025-09-11T04:58:07Z

aave-core/sources/aave-pool/pool_configurator.move

    use aave_pool::emode_logic;
    use aave_pool::pool;

+    friend aave_pool::gho_direct_minter;


This looks like a questionable/problematic design choice.

We should keep pool_configurator the single, top level caller of all protocol configuration setters, which means:

We should let gho_direct_minter be a friend to pool_configurator

Not pool_configurator a friend to gho_direct_minter.

matchv · 2025-09-11T05:12:50Z

aave-core/sources/aave-pool/pool_configurator.move

+    /// @param account The account signer of the caller
+    /// @param asset The address of the underlying asset of the reserve
+    /// @param new_supply_cap The new supply cap of the reserve
+    public(friend) fun set_supply_cap_internal(


A questionable design choice related to the above one.
This extend the ACL check to another file gho_direct_minter.move, even if the ACL check is present there after adding the assert, this function is generic and can target any asset, nothing in this file constrains it is to GHO. If someone later refactors the gho_direct_minter with a weakened ACL, it can mutate a top level risk parameter from there, and the configurator can't stop it.

Again I suggest that we keep all the top level setters in this file, not in other files.

matchv · 2025-09-11T05:21:45Z

aave-core/sources/gho-direct-minter/gho_direct_minter.move

+        is_risk_admin(gho_reserve_entity);
+
+        // check: the gho_reserve_entity must be registered as an entity with a non zero entity limit
+        let (_, _) = ensure_entity_gho_usage(gho_reserve_entity);


let's use these return values, after (limit, used), assert amount <= limit - used

matchv · 2025-09-11T05:22:30Z

aave-core/sources/gho-direct-minter/gho_direct_minter.move

+        is_risk_admin(gho_reserve_entity);
+
+        // check: the gho_reserve_entity must be registered as a entity with a non zero limit
+        let (_, _) = ensure_entity_gho_usage(gho_reserve_entity);


after (limit, used), assert amount <= limit - used

matchv · 2025-09-11T05:29:18Z

aave-core/sources/aave-tokens/a_token_factory.move

+        metadata_address: address
+    ) acquires TokenMap {
+        transfer_on_liquidation(from, to, amount, index, metadata_address);
+    }


This function can transfer aTokens from any address without signature when invoked by a friend. Today you only transfer from the minter’s own address, but the capability is unrestricted.

Consider to replace "friend" exposure with a narrower helper in a_token_factory, e.g. transfer_from_self_to(address to, ...) that internally asserts from == <this module’s signer address> or accepts only the minter’s signer address returned by get_direct_minter_signer_address(). Alternatively, keep friend but in transfer_excess_to_treasury assert that from == get_direct_minter_signer_address().

matchv

In my opinion we should put gho_direct_minter in the GHO repo, not in aptos-aave-v3.

Why this is better

Ownership/governance fit: The direct minter is a GHO facilitator component. Keeping it under the GHO codebase aligns upgrades and reviews with GHO governance, mirroring the Solidity design and deployment model described in the original docs and contract README and GhoDirectMinter.sol.
Security boundary: Aave Core should remain asset-agnostic. Housing GHO-specific facilitator logic in Aave forces “friend” surfaces or risk-parameter mutations that widen the trusted set. Keeping minter in GHO avoids adding Aave-internal hooks or friends.
Upgrade/dependency hygiene: Decouples GHO facilitator releases from protocol-core versioning. The minter can import the pool’s public APIs and evolve independently, without needing Aave to expose specialized internal setters.
1:1 with Ethereum architecture: On Ethereum, facilitators live outside the pool; the pool remains unchanged. This matches the cross-chain GSM/GHO strategy outlined in the explainer and minter repo.

matchv · 2025-09-11T20:02:19Z

aave-core/sources/gho-direct-minter/gho_direct_minter.move

+    // Constants
+    /// @notice Name for the GHO direct minter object
+    const GHO_DIRECT_MINTER_NAME: vector<u8> = b"GHO_DIRECT_MINTER";
+    /// @notice seed for deriving the GHO reserve address
+    const GHO_RESERVE_SEED: vector<u8> = b"GHO_RESERVE";


If I understand our design correctly, we suppose to have a dedicated GhoReserve per facilitator, so we may want to move these seeds into GhoDirectMinterData instead of hardcoded global seeds for every deployment.

matchv · 2025-09-11T20:05:31Z

aave-core/sources/gho-direct-minter/gho_direct_minter.move

+
+        // temporarily set the supply cap to 0 to disable it while supplying
+        let old_supply_cap =
+            pool::get_reserve_configuration(gho_direct_minter_data.gho_token_address).get_supply_cap();


We typically use reserve_config::get_supply_cap(&map) in the core protocol, can we unify the method here?

matchv · 2025-09-11T20:21:06Z

aave-core/sources/gho-direct-minter/gho_direct_minter.move

+        );
+
+        // extract the entity level
+        let (_, used) = ensure_entity_gho_usage(gho_reserve_entity);


This will throw with a limit == 0, do we have such possible scenario? that DAO set the limit == 0 and someone still wants to transfer_excess_to_treasury?

matchv · 2025-09-11T20:23:40Z

aave-core/sources/gho-direct-minter/gho_direct_minter.move

+    struct GhoDirectMinterData has key {
+        gho_token_address: address,
+        extend_ref: ObjExtendRef,
+        transfer_ref: ObjectTransferRef,


transfer_ref is defined and init, but not used anywhere, is it intended?

matchv · 2025-09-11T20:25:44Z

aave-core/sources/gho-direct-minter/gho_direct_minter.move

+    /// @notice Ensures the caller is either the pool owner or the GHO Guardian
+    /// @dev Reverts otherwise
+    /// @param account The signer to check
+    fun only_owner_or_guardian(account: &signer) {
+        let caller = signer::address_of(account);
+        assert!(
+            caller == @aave_pool || is_gho_guardian(caller),
+            error_config::get_ecaller_not_gho_guardian()
+        );
+    }


Defined but unused

matchv · 2025-09-11T20:31:09Z

aave-core/sources/gho-direct-minter/gho_direct_minter.move

+    /// @dev Asserts that the GHO direct minter resource exists
+    inline fun assert_gho_reserve_exists() {
+        assert!(
+            exists<GhoDirectMinterData>(gho_direct_minter_address()),
+            error_config::get_eresource_not_exist()
+        );
+    }


Maybe rename this function according to our intention?

gho_direct_minter_address(): the object address that stores GhoDirectMinterData (the minter’s own state).

gho_reserve::get_gho_reserve_address(GHO_RESERVE_SEED): the GHO Reserve’s address (where bridge-minted GHO is held and entity usage is tracked).

mpsc0x self-assigned this Sep 2, 2025

mpsc0x requested review from matchv and meng-xu-cs as code owners September 2, 2025 06:21

mpsc0x force-pushed the feat/gho-direct-minter branch 10 times, most recently from de27cba to d2332c4 Compare September 2, 2025 22:31

feat: added gho direct minter

8881321

mpsc0x force-pushed the feat/gho-direct-minter branch 2 times, most recently from 9ece938 to 571b250 Compare September 3, 2025 12:15

feat: added atoken proper transfer functionality

5b55f32

mpsc0x force-pushed the feat/gho-direct-minter branch from 571b250 to 5b55f32 Compare September 3, 2025 13:21

added git token for unit tests

5105a57

mpsc0x force-pushed the feat/gho-direct-minter branch from f2f59b3 to 5105a57 Compare September 4, 2025 10:41

added remaining gho workflow integrations

dbcbca0

mpsc0x force-pushed the feat/gho-direct-minter branch from d7a950c to dbcbca0 Compare September 4, 2025 11:23

skynnes added 3 commits September 4, 2025 14:32

Update typescript-integration-tests.yml

ea1af6e

fix: add token to git config for actions workflow

fix: testnet-deployment action

df76d39

fix: Build documentation Action

ea73843

skynnes temporarily deployed to github-pages September 4, 2025 13:39 — with GitHub Actions Inactive

mpsc0x force-pushed the feat/gho-direct-minter branch from efe6ef7 to 24a65fd Compare September 4, 2025 14:22

mpsc0x temporarily deployed to github-pages September 4, 2025 14:26 — with GitHub Actions Inactive

dfix: examples github action

5ec5926

mpsc0x force-pushed the feat/gho-direct-minter branch from 24a65fd to 5ec5926 Compare September 5, 2025 08:58

mpsc0x temporarily deployed to github-pages September 5, 2025 09:02 — with GitHub Actions Inactive

feat: added getter for gho_reserve

87f856a

matchv requested changes Sep 11, 2025

View reviewed changes

mpsc0x closed this Sep 14, 2025

feat: added gho direct minter #57

feat: added gho direct minter #57

Uh oh!

Conversation

mpsc0x commented Sep 2, 2025

Uh oh!

codecov bot commented Sep 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

matchv left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

codecov bot commented Sep 4, 2025 •

edited

Loading

matchv left a comment •

edited

Loading