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

Deprecated host functions that were supported in early releases of the substrate make a node unable to synchronize and verify the history from the genesis block. #4338

Open
2 tasks done
yahortsaryk opened this issue Apr 30, 2024 · 4 comments
Labels
I2-bug The node fails to follow expected behavior. I10-unconfirmed Issue might be valid, but it's not yet known.

Comments

@yahortsaryk
Copy link

yahortsaryk commented Apr 30, 2024

Is there an existing issue?

  • I have searched the existing issues

Experiencing problems? Have you tried our Stack Exchange first?

  • This is not a support question.

Description of bug

Hello, recently I posted this question at the Stack Exchange that describes the issue in detail.

To summarize, the removal of the sandboxing host function interface in PR #12852 causes old transactions submitted to the pallet-contracts to fail while synchronizing a node with an old chain from the genesis block in full mode and verifying the blocks history. At some point in time, the sandbox was an internal dependency of the pallet-contracts. This requires the function executor to have the sandbox-related host functions available at the host to process old transactions.

I tried to resolve the issue by extending the default host functions with sandbox ones to process old blocks. For that, I literally copied a part of the code for the wasmi FunctionExecutor to my network's node source code at the state it was in polkadot-v0.9.33 release. But it didn't do the trick. Executing old transactions using this "custom" function executor caused Storage root must match that calculated error. Probably, this attempt is not a viable solution at all, as I can't apply the correct arguments (MemoryRef, TableRef, etc.) to the "custom" function executor due to accessibility modifiers in substrate crates.

It feels like there is no way to fix the issue without a full fork of the substrate framework, where I can return the sandboxing host function interface back for the newest substrate releases.
Unless there is a way to recover the synchronization with old chain in full mode from the genesis block for substrate-based nodes that are built on top of the substrate-node-template, I think this issue should be treated as a bug in the substrate.

Steps to reproduce

  1. Compile a substrate-based node that includes pallet-contracts and is built on top of substrate 2.0.1 release. Run the node and start a network.
  2. Deploy a smart contract and execute some calls in the deployed smart contract.
  3. Upgrade the node with any recent substrate starting from v0.9.35 release.
  4. Run a second node with the upgraded substrate version and let it synchronize with the network in full mode starting from the genesis block.
@yahortsaryk yahortsaryk added I10-unconfirmed Issue might be valid, but it's not yet known. I2-bug The node fails to follow expected behavior. labels Apr 30, 2024
@bkchr
Copy link
Member

bkchr commented Apr 30, 2024

For that, I literally copied a part of the code for the wasmi FunctionExecutor to my network's node source code at the state it was in polkadot-v0.9.33 release

Yeah this is the right idea.

Probably, this attempt is not a viable solution at all, as I can't apply the correct arguments (MemoryRef, TableRef, etc.) to the "custom" function executor due to accessibility modifiers in substrate crates.

I don't get the problem? I mean you implemented the host functions, but you could not implement them as before? Why?

@yahortsaryk
Copy link
Author

yahortsaryk commented Apr 30, 2024

Thank you for your quick answer (cc: @bkchr) !

I don't get the problem? I mean you implemented the host functions, but you could not implement them as before? Why?

It looks like I implemented them exactly as they were in polkadot-v0.9.33 release, as I just copied them and resolved some simple compilation issues with imports without any custom code.
But I'm not sure, what exact arguments I need to apply to this "custom" function executor to let it process the old transactions, and how to obtain those arguments properly. It looks like quite internal stuff, and I suppose that I may have a lack of access to the required crates's members (due to accessibility modifiers) to construct the arguments list properly.

Originally the FunctionExecutor type accepts:

impl FunctionExecutor {
	fn new(
		m: MemoryRef,
		heap_base: u32,
		t: Option<TableRef>,
		host_functions: Arc<Vec<&'static dyn Function>>,
		allow_missing_func_imports: bool,
		missing_functions: Arc<Vec<String>>,
	) -> Result<Self, Error> {
...
	}
}

What values should I use for the m, heap_base, t, and other parameters, and how to obtain them assuming that my node is built on top of the substrate-node-template?

If you take a look at my attempt to fix the issue, I'm creating the function executor using a blob of my network's runtime. Is it correct or not? If yes, should I use the blob of the runtime that was running when the missing sandboxing functions were applied (the Mainnet that was ~3 years ago), or should I just use the blob of the latest runtime (the current Mainnet) to create the instance of the function executor?

If you could shed some light on what arguments I need to use to instantiate my "custom" FunctionExecutor to let it process transactions from old blocks, It would be really helpful.

The reason I thought the "custom" function executor was not a viable solution is that when I tried to process the history using the copy-pasted functions, I got the Storage root must match that calculated error.
I assumed this error happens because I'm passing incorrect MemoryRef, or TableRef, or any other arguments to the function executor instance, and that causes some mismatch in state verification after processing an old transaction. But if you are saying this is the correct approach, I'm no longer sure about this assumption.

Can you explain how to properly construct and inject an instance of the custom FunctionExecutor that will be used specifically to process old blocks/transactions with calls to deprecated sandboxing host functions?

@bkchr
Copy link
Member

bkchr commented May 2, 2024

Your problem is stuff like this:

https://github.com/Cerebellum-Network/blockchain-node/blob/974e1910c9486258525ba09c1aa0fae49551fed8/node/runtime-interfaces/src/wasmi_executor.rs#L182

self.memory is not the memory of your runtime.

       pub fn memory_set(
		&mut self,
		memory_id: MemoryId,
		offset: WordSize,
                data: &[u8],
	) -> WResult<u32> {
		let sandboxed_memory =
			self.sandbox_store.borrow().memory(memory_id).map_err(|e| e.to_string())?;

		let len = val_len as usize;

		if sandboxed_memory.write_from(Pointer::new(offset as u32), &data).is_err() {
			return Ok(ERR_OUT_OF_BOUNDS)
		}

		Ok(ERR_OK)
	}
	fn memory_set(
		&mut self,
		memory_idx: u32,
		offset: u32,
		val_ptr: Pointer<u8>,
		val_len: u32,
	) -> u32 {
		let lock = SANDBOX.lock();
		let mut sandbox = lock.borrow_mut();
 
               let data = self.read_memory(val_pr, size).unwrap();

		sandbox
			.memory_set(memory_idx, offset, data)
			.expect("Failed to set memory with sandbox")
	}

Writing it this way should make it work. You will also need to change memory_get in a similar way.

@yahortsaryk
Copy link
Author

Thank you for your clarifications, I will try and let you know whether it works or not and collect new details if it doesn't :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I2-bug The node fails to follow expected behavior. I10-unconfirmed Issue might be valid, but it's not yet known.
Projects
None yet
Development

No branches or pull requests

2 participants