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

Cargo Stylus Factory #240

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Cargo Stylus Factory #240

wants to merge 1 commit into from

Conversation

chrisco512
Copy link

Draft for a Stylus factory contract that may be used by the cargo stylus CLI tool to deploy, activate, and run an arbitrary constructor in a single transaction.

In Stylus/WASM context we are unable to run Rust or WASM code during the deployment step. There is also a need to activate WASM programs, which is currently abstracted from users via the cargo stylus tool. Users can currently define in their Rust code an "init" method but it must be called after activation and there is a need to gate the function manually to a specific deployer address as well as add arbitrary state variables to gate other methods from being invoked before the contract is initialized.

This contract enables a better developer experience. The intention is to deterministically deploy this contract using create2 on Arbitrum chains so that Cargo Stylus can use it to deploy. The Stylus SDK will add a future #[constructor] proc macro that will gate a defined method to being invoked by this contract alone during the deployment transaction.

@cla-bot cla-bot bot added the s label Aug 26, 2024
Comment on lines +4 to +8
interface IArbWasm {
function activateProgram(
address program
) external payable returns (uint16 version, uint256 dataFee);
}
Copy link
Member

Choose a reason for hiding this comment

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

should import from ./src/precompiles/ArbWasm.sol

Comment on lines +47 to +51
// Function to deploy and init a new contract
// NOTE: This function should only be invoked if
// ArbWasm's codehashVersion(bytes32 codehash) method returns the
// current stylusVersion(), verify this locally before invoking
function deployInit(
Copy link
Member

@gzeoneth gzeoneth Aug 27, 2024

Choose a reason for hiding this comment

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

instead of having a separate method, we can just conditional activate based on the result of codehashVersion in the other method
can also conditional on if the activation cost provided is 0

// Initialize the contract
initializeContract(newContractAddress, _constructorCalldata, msg.value);

return newContractAddress;
Copy link
Member

Choose a reason for hiding this comment

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

excess value not refunded (some constructor might refund callvalue)

) external payable returns (uint16 version, uint256 dataFee);
}

contract CargoStylusFactory {
Copy link
Member

@gzeoneth gzeoneth Aug 27, 2024

Choose a reason for hiding this comment

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

missing a payable receive function to receive refund excess fund provided for activation

Comment on lines +78 to +110
function deployContract(bytes memory _bytecode) internal returns (address) {
address newContractAddress;

assembly {
newContractAddress := create(0, add(_bytecode, 0x20), mload(_bytecode))
}

if (newContractAddress == address(0)) {
revert ContractDeploymentError(_bytecode);
}

emit ContractDeployed(newContractAddress, msg.sender);

return newContractAddress;
}

// Internal function to activate a Stylus contract
function activateContract(
address _contract,
uint256 _activationValue
) internal returns (uint256) {
(, uint256 dataFee) = IArbWasm(ARB_WASM).activateProgram{value: _activationValue}(
_contract
);

emit ContractActivated(_contract);

return dataFee;
}

// Internal function to initialize a Stylus contract by
// invoking its constructor
function initializeContract(
Copy link
Member

Choose a reason for hiding this comment

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

if we were to consolidate the 2 public function, we might as well inline these instead of making internal functions

Comment on lines +40 to +42
initializeContract(newContractAddress, _constructorCalldata, _constructorValue);

refundExcessValue(activationFee, _constructorValue);
Copy link
Member

Choose a reason for hiding this comment

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

this ignore any refund from the constructor call (if any), I think we can wrap a balance check before and after the call to constructor to know the net amount spent in the constructor and do refund appropriately

there is also an additional edge case where the constructor cost is negative (if it somehow received more fund from the constructor)

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

Successfully merging this pull request may close these issues.

2 participants