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

Make sol-dbg more browser friendly #1

Open
cd1m0 opened this issue Jun 6, 2022 · 6 comments
Open

Make sol-dbg more browser friendly #1

cd1m0 opened this issue Jun 6, 2022 · 6 comments

Comments

@cd1m0
Copy link
Collaborator

cd1m0 commented Jun 6, 2022

Currently sol-dbg is mostly tested with node. There are several tasks to make it more browser friendly:

  1. Replace all uses of Buffer with Uint8Array. Buffer is supported natively in nodejs, but is a polyfill in most browsers. Instead we should use the more cross-platform Uint8Array
  2. Remove the dependency on remix-lib. We currently only need remix-lib for the WebVMProvider wrapper class. One idea is to just clone that file and fill out more of its implementation. There may be better ways to do this too.
@blitz-1306
Copy link
Contributor

blitz-1306 commented Jun 7, 2022

I guess WebVMProvider is now VmProxy, judging by recent commit. Also consider following: https://github.com/ethereum/remix-project/tree/master/libs/remix-simulator

@cd1m0
Copy link
Collaborator Author

cd1m0 commented Jun 7, 2022

I guess WebVMProvider is now VmProxy, judging by recent commit. Also consider following: https://github.com/ethereum/remix-project/tree/master/libs/remix-simulator

Yep. You're right. I am not sure if just copying it is the cleanest thing to do? Anothering thing we could do is to remove the web just expose the raw vm in the SolTxDebugger and move the remix-lib dependency into the dev dependencies.

@blitz-1306
Copy link
Contributor

blitz-1306 commented Jun 8, 2022

Regarding migration to Uint8Array from Buffer: I would be able to change our codebase, but some of our dependencies, like Address of ethereumjs-util, are still using Buffer. So I guess polyfill will be there anyway.

@blitz-1306
Copy link
Contributor

blitz-1306 commented Jun 8, 2022

Also this seem to mean, that we want to require developer to add vm as a separate script to not ship/bundle it on our own. I took some time and currently thinking that if we even try to move to Uint8Array - we will "shoot our leg" with the bugs and vm.js friendliness (due to Address, keccak256 and some other funcs are requiring Buffer as an input).

@cd1m0
Copy link
Collaborator Author

cd1m0 commented Jun 8, 2022

This is a good point. So lets drop the conversion to Uint8Array, as it doesn't actually remove the polyfill dependency anyway, and just adds extra conversion code.

@blitz-1306
Copy link
Contributor

blitz-1306 commented Jun 10, 2022

I made #3 to address some issues with Web3VMProvider. Simulator is a bit more minimalistic to be used for that purpose. @cd1m0, please, consider reviewing. Thanks.

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

No branches or pull requests

2 participants