Skip to content
This repository has been archived by the owner on Jul 9, 2021. It is now read-only.

ERC1155 Proxy: Improved Readability #1826

Closed
wants to merge 1 commit into from
Closed

ERC1155 Proxy: Improved Readability #1826

wants to merge 1 commit into from

Conversation

hysz
Copy link
Contributor

@hysz hysz commented May 21, 2019

Description

In this PR:

  1. Load from calldata into memory only the components of asset data that will remain unchanged
  2. Read ERC1155 asset address from calldata, rather than memory.
  3. The offset to asset data includes 4 bytes for the selector, simplifying some calculations and making it consistent with other offsets.

Testing instructions

Types of changes

Checklist:

  • Prefix PR title with [WIP] if necessary.
  • Add tests to cover changes as needed.
  • Update documentation as needed.
  • Add new entries to the relevant CHANGELOG.jsons.

@hysz hysz requested a review from abandeali1 as a code owner May 21, 2019 19:41
@coveralls
Copy link

coveralls commented May 21, 2019 •
edited
Loading

Coverage Status

Coverage remained the same at 84.231% when pulling 2b7e7d1 on fix/contracts/erc1155AssetProxyReadability into a8451d5 on fix/contracts/erc1155AssetProxyScaledValues.

@hysz hysz force-pushed the fix/contracts/erc1155AssetProxyScaledValues branch 2 times, most recently from def6efd to ba596d5 Compare May 22, 2019 00:05
@hysz hysz force-pushed the fix/contracts/erc1155AssetProxyReadability branch from 35a29bc to 5a25b02 Compare May 22, 2019 00:08
@hysz hysz force-pushed the fix/contracts/erc1155AssetProxyScaledValues branch 3 times, most recently from 5751b80 to 8013b57 Compare May 22, 2019 15:11
@hysz hysz force-pushed the fix/contracts/erc1155AssetProxyReadability branch from 5a25b02 to d29dc9d Compare May 22, 2019 16:03
@hysz hysz force-pushed the fix/contracts/erc1155AssetProxyReadability branch from d29dc9d to 2b7e7d1 Compare May 22, 2019 16:07
@hysz hysz mentioned this pull request May 27, 2019
Merged
4 tasks
@hysz
Copy link
Contributor Author

hysz commented May 30, 2019

This PR has been superseded by #1837

@hysz hysz closed this May 30, 2019
This was referenced Jan 18, 2022
Open
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@abandeali1 abandeali1 abandeali1 approved these changes

Assignees
No one assigned
Labels
contracts
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@hysz @coveralls @abandeali1