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

Soroban: Support for Constructors #1675

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

tareknaser
Copy link
Contributor

This PR adds support for user-defined constructors in Soroban, specifically for constructors with no arguments. Note that constructors with arguments are not yet supported (see comments for details).
Part of #1672

Other changes include:

  • Refactored code in tests/soroban.rs to allow providing constructors when registering contracts on the chain.
  • Added tests for constructors in tests/soroban_testcases/constructor.rs.

…tructors

Signed-off-by: Tarek <tareknaser360@gmail.com>
…n logic

Signed-off-by: Tarek <tareknaser360@gmail.com>
Signed-off-by: Tarek <tareknaser360@gmail.com>
Signed-off-by: Tarek <tareknaser360@gmail.com>
Copy link
Contributor Author

@tareknaser tareknaser left a comment

Choose a reason for hiding this comment

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

So adding support for constructors with custom arguments wasn't straightforward. I have two approaches in mind that I thought of, which I’ll explain below.

Let me know what you think, or if there’s a better, more straightforward approach I might be missing.

Comment on lines +299 to +307
// call the user defined constructor (if any)
if let Some(cfg_no) = constructor_cfg_no {
let constructor = binary.functions[cfg_no];
let constructor_name = constructor.get_name().to_str().unwrap();
binary
.builder
.build_call(constructor, &[], constructor_name)
.unwrap();
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can set cfg.params to the parameters of the constructor FunctionValue's params. This will likely include a mapping that looks something like:

let mut params = vec![];
for param in constructor.get_params() {
	let param = match param.get_type() {
		BasicTypeEnum::IntType(int_type) => {
			let ty = match int_type.get_bit_width() {
				1 => ast::Type::Bool,
				32 => ast::Type::Uint(32),
				64 => ast::Type::Uint(64),
				_ => unreachable!(),
			};
			ast::Parameter::new_default(ty)
		}
		_ => unreachable!(),
	};
	params.push(param);
}
cfg.params = sync::Arc::new(params);

This won’t work because we still need to pass the arguments to build_call() (on line 296), which, of course, we don’t know.

Comment on lines 65 to 67

Self::emit_initializer(&mut binary, ns);
Self::emit_initializer(&mut binary, ns, contract.constructors(&ns).first());

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Another approach I have in mind is to get the constructor ControlFlowGraph itself and prepend it with a call to storage_initializer, but this won't work either because the contract is immutable.

We could clone it and re-export it, but I’m not sure if that’s the best approach.

Signed-off-by: Tarek <tareknaser360@gmail.com>
@tareknaser tareknaser force-pushed the soroban_no_args_constr branch from 87ab13a to 4f2d6de Compare November 30, 2024 10:48
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.

1 participant