There is no way to enable transfer for all users without upgrading the contract.
It would be good to add a Boolean flag and let the owner disable or enable public transfer.

It's necessary to check if Uniswap pair tokens are USDC and PION, reserves are zero or not and also check that the boost amount is greater than zero.

Inheritance from ERC20Upgradeable is redundant because it’s already inherited in the ERC20BurnableUpgradeable contract.

contract Token is
    Initializable,
    ERC20Upgradeable,
    ERC20BurnableUpgradeable,
    PausableUpgradeable,
    AccessControlUpgradeable
{

The same is true for ERC721.

contract BondedToken is
    Initializable,
    ERC721Upgradeable,
    PausableUpgradeable,
    AccessControlUpgradeable,
    OwnableUpgradeable,
    ERC721BurnableUpgradeable
{

It is more as efficient to not update storage when lockedOf[tokenIdA][tokensWhitelist[i]] is 0

Do you think we won't need a setter for nodeManager?

All state variables should be named using mixed case style like tokensWhitelist.

uint256 public _tokenIdCounter;

address public Token;

address public treasury;

address[] public tokensWhitelist;

Is it intended for VePION to permit locking tokens under NFTs owned by others?

We should enable a service on veNFTs to transfer to other wallets via customer support by muon project (as long as the transfer on NFTs is disabled)

We have this assertion on merge and split:

require(ownerOf(tokenIdA) == msg.sender, "Not Owned");

and only the owner of the NFT can call these functions.

There is no way to approve and let another contract call them.

( we do not need to implement it. we need to discuss and make a decision )

What is the purpose of saving transferWhitelist? It is not used anywhere.

Add newTokenId here

It's better and more gas efficient to merge 3 amount validations and validate it before Muon signature verification.

ownerOf(tokenB) should be !=address(0)

Here, duplicate tokens could be pushed to tokensWhitelist.

We have multiple platforms(ALICE, PION, MUON) and the code base should support all of them.

Something like this:

contract veToken{
 // existing codes
}


contract vePION is veToken{
  // customize name, symbol, ...
}

contract veALICE is veToken{
 // ...
}

Treasury should never be 0x0

on events

I have a couple of questions regarding the current implementation of Minter:

1. Minting Reversion: Instead of doing nothing, would it be better to revert the minting if the mintPeriod is not over yet? This would provide more transparent behaviour feedback.

2. PION Address Setter: Could we implement a setter function for the PION address?

This is more gas efficient:

if(!isPublicTransferEnabled){
            require(
                    from == address(0) ||
                   to == address(0) ||
                    hasRole(TRANSFERABLE_ADDRESS_ROLE, from) ||
                    hasRole(TRANSFERABLE_ADDRESS_ROLE, to),
                "Transfer is Limited"
            );
}
// rest of codes

Why should the contract address be in the transfer whitelist?

The contract is burnable. If the users burn tokens, totalLocked will be invalid.

If we assume that tokens are normal ERC20 tokens (no tax, reflection, ...), it is better to compare the receivedAmount and amounts[i] and reject the transaction if they are not equal.

whitelist contract address for transfering

We want to grant access to a limited list of wallets and it is a good use case for AccessControl.

There could be 2 roles:

OWNER_ROLE -- no need to use Ownable anymore
TRANSFER_WHITELIST_ROLE

This seems more standard.

In the current code, there is no way to remove a wallet from the whitelist.

Why not using _burn? whats the difference between _mint and _burn?

I think it’s more gas efficient if we first load the nodes[nodeId] into a variable and then update it. Do you think so?

Otherwise the balance will be 0

Using the split function, it is possible to transfer all of the locked tokens from tokenA to tokenB with another owner.
It is a kind of bypassing the transfer limits.

We discussed and mentioned that there is no need to have more than one owner/manager.
Let's keep the contract ownable and use onlyOwner for the functions that are using onlyRole(MANAGER_ROLE).

So there will be only TRANSFERABLE_ADDRESS_ROLE.

It seems that getNode() is unused. Do we need it?