Skip to content

Marco/mp 1027 template design merchanttokenfactory contract#9

Merged
Astronaut828 merged 29 commits intomainfrom
marco/mp-1027-template-design-merchanttokenfactory-contract
Feb 11, 2025
Merged

Marco/mp 1027 template design merchanttokenfactory contract#9
Astronaut828 merged 29 commits intomainfrom
marco/mp-1027-template-design-merchanttokenfactory-contract

Conversation

@Astronaut828
Copy link
Contributor

No description provided.

@linear
Copy link

linear bot commented Jan 10, 2025

error UNAUTHORIZED();
error INVALID_ADDRESS();

contract MerchantToken is MerchantFactory {
Copy link

@its-everdred its-everdred Jan 10, 2025

Choose a reason for hiding this comment

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

🤔 I know this MerchantToken is MerchantFactory relationship is borrowed from our previous factories, but it's always thrown me for a loop. Since this is our Nth factory, I think it's worth reassessing the interface between these two contracts.

In my head, I imagine a Factory deploying Tokens, but the way it's written at the moment, these are one and the same and we've just broken up the code somewhat arbitrarily between two files.

Let's either change the relationship between these two contract files, or just consolidate them and break out a helper contract along a different boundary

Copy link

@its-everdred its-everdred Jan 10, 2025

Choose a reason for hiding this comment

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

one specific refactor i'd suggest, we have 3 deploy related functions:

  1. MerchantToken.deployToken()
  2. MerchantFactory.deploy()
  3. MerchantFactory._deploy()

These could be consolidated into a single MerchantFactory.deploy() with helper functions to break it up as necessary. At the moment, we don't even call MerchantFactory.deploy() so that code + MerchantFactory._deploy() never get used.

uint256 _lpReserve,
uint256 _airdropReserve,
uint256 _rewardsReserve
) external returns (InstantLiquidityToken token) {
Copy link

@its-everdred its-everdred Jan 14, 2025

Choose a reason for hiding this comment

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

For reference, here are the old /airdrop params:

{
  "network": "base",            // not necessary in this function
  "symbol": "METALTEST",     // included _symbol
  "initialPrice": 0.001,          // now passed in via createLiquidityPool()
  "name": "metal test",           // included _name
  "totalSupply": 1000000000,  // included _totalySupply
  "recipient": {
    "address": "0x000000000000000000000000000000000000dEaD", // included _merchant
    "supply": 4000000 // included _merchantAmount
  },
  "airdrop": {
    "addresses": [
      "0x0000000000000000000000000000000000000001",
      "0x0000000000000000000000000000000000000002",
      "0x0000000000000000000000000000000000000003"
    ], // will now be passed in via direct gaslite call in metal-service
    "supply": 8000000 // included _airdropReserve
  }
}

New fields:

_rewardsReserve 
_lpReserve

looks good 👍

Copy link

@its-everdred its-everdred Jan 14, 2025

Choose a reason for hiding this comment

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

I know we've used the name merchant internally, but I'm not sure I like it in this context. these will eventually deployable by the users of the merchant too. what about creator instead? that'll be accurate whether it's a merchant or a user of the merchant.

if (_rewardsReserve > 0) {
merchantTransfer(address(token), signer, _rewardsReserve);
}

Choose a reason for hiding this comment

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

Suggested change
// lpReserve remains on the factory until createLiquidityPool is called

// proxy swap
INonfungiblePositionManager(0x56c65e35f2Dd06f659BCFe327C4D7F21c9b69C2f);
}
}
Copy link

@its-everdred its-everdred Jan 14, 2025

Choose a reason for hiding this comment

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

function _getAddresses() can be moved to Constants.sol or a new helper contract since it shouldn't change and isn't specific to this factory.

@Astronaut828 Astronaut828 merged commit 4c4bc94 into main Feb 11, 2025
1 check 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.

2 participants