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

Add CallProxies for mainnet addresses on alfajores #152

Closed
wants to merge 1 commit into from

Conversation

palango
Copy link

@palango palango commented Jun 17, 2024

Ref #141

Moving state accounts including storage turned out to be not supported. As we currently only need this for testnets, this offers an alternative solution instead.

This PR creates a new proxy which used call to forward calls to the implementation contract. It should only be used for proxying mainnet addresses to the respective (normal) proxy on alfajores.

@pahor167 Please give me some early feedback on this approach 🙏

pahor167
pahor167 previously approved these changes Jun 17, 2024
returndatacopy(0, 0, returndatasize())

switch result
// delegatecall returns 0 on error.

Choose a reason for hiding this comment

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

Suggested change
// delegatecall returns 0 on error.
// call returns 0 on error.

implementation = _implementation;
}

function _delegate(address _implementation) internal virtual {

Choose a reason for hiding this comment

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

can we rename it to something like _call since we are not actually delegating?

@pahor167
Copy link

Ref #141

Moving state accounts including storage turned out to be not supported. As we currently only need this for testnets, this offers an alternative solution instead.

This PR creates a new proxy which used call to forward calls to the implementation contract. It should only be used for proxying mainnet addresses to the respective (normal) proxy on alfajores.

@pahor167 Please give me some early feedback on this approach 🙏

This is a good workaround

@pahor167 pahor167 dismissed their stale review June 17, 2024 12:57

There might be a problem with sending ETH

@palango
Copy link
Author

palango commented Jun 18, 2024

This doesn't work, the approach fails for payable contracts.

@palango palango closed this Jun 18, 2024
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