Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: remove transmuter/overhaul transmutation #23

Merged
merged 2 commits into from
Oct 26, 2023
Merged

Conversation

0xTimepunk
Copy link
Contributor

Fixes partially https://github.com/code-423n4/2023-09-superform/issues/73

Remainder of fix in superform-core repo

src/ERC1155A.sol Outdated Show resolved Hide resolved
src/ERC1155A.sol Outdated Show resolved Hide resolved
src/ERC1155A.sol Outdated Show resolved Hide resolved
src/ERC1155A.sol Outdated Show resolved Hide resolved
src/ERC1155A.sol Outdated Show resolved Hide resolved
src/ERC1155A.sol Outdated Show resolved Hide resolved
@sujithsomraaj sujithsomraaj self-requested a review October 26, 2023 16:14
@0xTimepunk 0xTimepunk merged commit 23159a2 into main Oct 26, 2023
2 checks passed
@0xTimepunk 0xTimepunk deleted the overhaul-erc1155a branch October 26, 2023 16:16

return synthethicTokenId[id];
}
function registerSERC20(uint256 id) external virtual override returns (address);
Copy link
Collaborator

@gpersoon gpersoon Oct 27, 2023

Choose a reason for hiding this comment

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

I assume you want to delegate the erc20 creating itself to another contract.
Even then it might still be good to keep the logic with synthethicTokenId[] inside this contract, to isolate it and to prevent mistakes.

if (synthethicTokenId[id] != address(0)) revert SYNTHETIC_ERC20_ALREADY_REGISTERED();
address syntheticToken = createToken(id); // done elsewhere
synthethicTokenId[id] = syntheticToken;
return synthethicTokenId[id];

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added

// If not, then check if the msg.sender has sufficient allowance
require(allowance(from, msg.sender, id) >= amount, "NOT_AUTHORIZED");
allowances[from][msg.sender][id] -= amount; // Deduct the burned amount from the allowance
require(allowance(from, operator, id) >= amount, "NOT_AUTHORIZED");
Copy link
Collaborator

Choose a reason for hiding this comment

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

string based error message (could be custom error)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

}

/// @dev handy helper to check if a SERC20 is registered
function _sERC20Exists(uint256 id) external view virtual returns (bool) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

external function but name starts with an _

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

/// @param id id of the ERC1155 to get the synthetic token address for
/// @return sERC20 address of the synthetic token for the given ERC1155 id
function getSyntheticTokenAddress(uint256 id) external view returns (address sERC20);

Copy link
Collaborator

Choose a reason for hiding this comment

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

could add function _sERC20Exists(uint256 id)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added

@@ -8,6 +8,7 @@ import { ERC20 } from "openzeppelin-contracts/contracts/token/ERC20/ERC20.sol";
/// @dev Synthetic ERC20 tokens out of 1155a
contract sERC20 is ERC20 {
address public immutable ERC1155A;
uint8 private immutable tokenDecimals;
Copy link
Collaborator

Choose a reason for hiding this comment

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

there is no interface file for sERC20 e.g. ISERC20.SOL, might be useful to have

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added

@@ -31,4 +33,8 @@ contract sERC20 is ERC20 {

_burn(owner, amount);
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

nitpick: spender could be operator in function burn

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants