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

Yash/pack #175

Merged
merged 68 commits into from
Jun 28, 2022
Merged

Yash/pack #175

merged 68 commits into from
Jun 28, 2022

Conversation

kumaryash90
Copy link
Member

No description provided.

@height
Copy link

height bot commented Jun 11, 2022

Link Height tasks by mentioning a task ID in the pull request title or commit messages, or description and comments with the keyword link (e.g. "Link T-123").

💡Tip: You can also use "Close T-X" to automatically close a task when the pull request is merged.

Comment on lines +210 to +233
function openPack(uint256 _packId, uint256 _amountToOpen)
external
nonReentrant
whenNotPaused
returns (Token[] memory)
{
address opener = _msgSender();

require(opener == tx.origin, "opener must be eoa");
require(balanceOf(opener, _packId) >= _amountToOpen, "opening more than owned");

PackInfo memory pack = packInfo[_packId];
require(pack.openStartTimestamp < block.timestamp, "cannot open yet");

Token[] memory rewardUnits = getRewardUnits(_packId, _amountToOpen, pack.amountDistributedPerOpen, pack);

_burn(_msgSender(), _packId, _amountToOpen);

_transferTokenBatch(address(this), _msgSender(), rewardUnits);

emit PackOpened(_packId, _msgSender(), _amountToOpen, rewardUnits);

return rewardUnits;
}

Check warning

Code scanning / Slither

Dangerous usage of `tx.origin`

Pack.openPack(uint256,uint256) (contracts/pack/Pack.sol#210-233) uses tx.origin for authorization: require(bool,string)(opener == tx.origin,opener must be eoa) (contracts/pack/Pack.sol#218)

Choose a reason for hiding this comment

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

@@ -75,8 +78,22 @@
address _to,
Token[] memory _tokens
) internal {
uint256 nativeTokenValue;

Check warning

Code scanning / Slither

Uninitialized local variables

TokenStore._transferTokenBatch(address,address,ITokenBundle.Token[]).nativeTokenValue (contracts/feature/TokenStore.sol#81) is a local variable never initialized

Choose a reason for hiding this comment

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

Comment on lines +26 to +404
Getter functions
//////////////////////////////////////////////////////////////*/

/// @dev Returns the underlying contents of a pack.
function getPackContents(uint256 _packId)
external
view
returns (Token[] memory contents, uint256[] memory perUnitAmounts)
{
PackInfo memory pack = packInfo[_packId];
uint256 total = getTokenCountOfBundle(_packId);
contents = new Token[](total);
perUnitAmounts = new uint256[](total);

for (uint256 i = 0; i < total; i += 1) {
contents[i] = getTokenOfBundle(_packId, i);
perUnitAmounts[i] = pack.perUnitAmounts[i];
}
}

/*///////////////////////////////////////////////////////////////
Internal functions
//////////////////////////////////////////////////////////////*/

/// @dev Returns whether owner can be set in the given execution context.
function _canSetOwner() internal view override returns (bool) {
return hasRole(DEFAULT_ADMIN_ROLE, _msgSender());
}

/// @dev Returns whether royalty info can be set in the given execution context.
function _canSetRoyaltyInfo() internal view override returns (bool) {
return hasRole(DEFAULT_ADMIN_ROLE, _msgSender());
}

/// @dev Returns whether contract metadata can be set in the given execution context.
function _canSetContractURI() internal view override returns (bool) {
return hasRole(DEFAULT_ADMIN_ROLE, _msgSender());
}

/*///////////////////////////////////////////////////////////////
Miscellaneous
//////////////////////////////////////////////////////////////*/

function generateRandomValue() internal view returns (uint256 random) {
random = uint256(keccak256(abi.encodePacked(_msgSender(), blockhash(block.number - 1), block.difficulty)));
}

/**
* @dev See {ERC1155-_beforeTokenTransfer}.
*/
function _beforeTokenTransfer(
address operator,
address from,
address to,
uint256[] memory ids,
uint256[] memory amounts,
bytes memory data
) internal virtual override {
super._beforeTokenTransfer(operator, from, to, ids, amounts, data);

// if transfer is restricted on the contract, we still want to allow burning and minting
if (!hasRole(TRANSFER_ROLE, address(0)) && from != address(0) && to != address(0)) {
require(hasRole(TRANSFER_ROLE, from) || hasRole(TRANSFER_ROLE, to), "restricted to TRANSFER_ROLE holders.");
}

if (from == address(0)) {
for (uint256 i = 0; i < ids.length; ++i) {
totalSupply[ids[i]] += amounts[i];
}
}

if (to == address(0)) {
for (uint256 i = 0; i < ids.length; ++i) {
totalSupply[ids[i]] -= amounts[i];
}
}
}

/// @dev See EIP-2771
function _msgSender()
internal
view
virtual
override(ContextUpgradeable, ERC2771ContextUpgradeable)
returns (address sender)
{
return ERC2771ContextUpgradeable._msgSender();
}

/// @dev See EIP-2771
function _msgData()
internal
view
virtual
override(ContextUpgradeable, ERC2771ContextUpgradeable)
returns (bytes calldata)
{
return ERC2771ContextUpgradeable._msgData();
}
}

Check warning

Code scanning / Slither

Missing inheritance

Pack (contracts/pack/Pack.sol#26-404) should inherit from IThirdwebContract (contracts/interfaces/IThirdwebContract.sol#4-19)
@@ -15,8 +15,11 @@
import "../lib/CurrencyTransferLib.sol";

contract TokenStore is TokenBundle, ERC721Holder, ERC1155Holder {
/// @dev The address interpreted as native token of the chain.
address public constant NATIVE_TOKEN = 0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE;

Check warning

Code scanning / Slither

Variable names too similar

Variable TokenStore.NATIVE_TOKEN (contracts/feature/TokenStore.sol#19) is too similar to TokenStore._transferTokenBatch(address,address,ITokenBundle.Token[])._nativeToken (contracts/feature/TokenStore.sol#90-95)
@joaquim-verges joaquim-verges merged commit 93c8815 into main Jun 28, 2022
@joaquim-verges joaquim-verges deleted the yash/pack branch June 28, 2022 22:50
Copy link

@GIgako19929 GIgako19929 left a comment

Choose a reason for hiding this comment

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

Approve

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.

3 participants