-
Notifications
You must be signed in to change notification settings - Fork 15
Contract deploy contract #439
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
base: main
Are you sure you want to change the base?
Conversation
… `ContractError`
595555c to
d107486
Compare
miloszm
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have some questions about:
- init function call logic
- new struct in the session
| init_arg: Option<Vec<u8>>, | ||
| owner: Vec<u8>, | ||
| gas_limit: u64, | ||
| call_init: bool, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why is the call_init mechanism introduced? I think init function needs to be called always if it is present, so far we have assumed that it is so: init called during deployment if present, there should not be a conditionality here
| engine: Engine, | ||
| inner: &'static mut SessionInner, | ||
| original: bool, | ||
| config: SessionConfig, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why is the SessionConfig field added here - I think when contract is deploying another contract via an abi, this abi should pass gas limit and price rather than having a session keeping it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's there to calculate the deploy charge:
let deploy_charge = max(
bytecode_len * env.gas_per_deploy_byte(),
env.min_deploy_points(),
);| .to_vec(), | ||
| ) | ||
| }; | ||
| env.deploy_raw( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why is this not calling init and init is called separately here below?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The call_inner function that deploy_raw uses seemed to be meant for top-level one-off calls.
Upon completion, the call tree and all instances are cleared. If an init function fails, we don't want the instance that is doing the deployment to be removed.
Aside from that, the memory snapshotting and the applying are things I don't think we need here.
It seemed clearer to me to just call init separately and only do the things that are needed.
For #160.