Skip to content

Complete set mirror#116

Open
nuevoalex wants to merge 2 commits intomainfrom
complete_set_mirror
Open

Complete set mirror#116
nuevoalex wants to merge 2 commits intomainfrom
complete_set_mirror

Conversation

@nuevoalex
Copy link
Collaborator

Roll our own share token to cut some of the gas cost incurred by using real complete set operations

Copy link
Member

Choose a reason for hiding this comment

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

It feels like we should fold this, the pool, and the router all into a single contract to minimize the amount of moving around assets we need to do? Or would we gain little benefit from such a combination?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yea thats the next thing I plan to take a stab at once this is in. Will involve a core contract that uses delegate calls for at least some of the implementation though bc of size constraints.

import { TokenId } from "./TokenId.sol";
import { IOICash } from "./IOICash.sol";

contract AugurConstantProductShareToken is ERC1155 {
Copy link
Member

Choose a reason for hiding this comment

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

Does this actually need to be an ERC1155? I think the minimum requirement is that a user can "roll their keys", but I don't know if we need full 1155 support? I'm not sure if it helps us to remove ERC-1155 dependency though.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It probably doesn't need the full ERC1155 but I think we wouldn't gain much by ripping out functionality. There shouldn't be much overhead of those features in its operation within our system. Its a good mental construct too for understanding how the system is operating and what state looks like.

Copy link
Member

Choose a reason for hiding this comment

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

If this and other contracts are cribbed from OpenZeppelin we should include a comment at the top indicating as much along with a link to the original. If they are not cribbed from OZ, then I think we should consider doing so.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

They're cribbed from AugurV2 which may have been cribbed from OZ. Will look.

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