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

I36 remove deps #37

Closed
wants to merge 69 commits into from
Closed

I36 remove deps #37

wants to merge 69 commits into from

Conversation

PopcornPaws
Copy link
Member

@PopcornPaws PopcornPaws commented May 14, 2021

Removed many dependencies in order to resolve #36

This PR possibly resolves #32 as well (this might need more testing)

Mark Melczer and others added 30 commits May 5, 2021 08:40
- faster if I just test this crate alone.
- tried to implement an umbrella trait for them, but for some reason,
  that didn't work.
- it might be worth investigating why tho
- however, when constructing an `EthereumType` from an integer, a
  reference to an integer is required which might be a bit inconvenient.
@PopcornPaws PopcornPaws requested a review from PumpkinSeed May 21, 2021 08:18
@PopcornPaws PopcornPaws marked this pull request as ready for review May 21, 2021 08:43
///
/// # Panics
///
/// Panics if the input data doesn't fit into the type, i.e. `N < L`.
Copy link
Member

Choose a reason for hiding this comment

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

Isn't it a prob;em in case of wasm? I guess in case of panic the caller side won't be notified about the panic, isn't it?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I guess, that's right, but this function was specifically written like this. The aim is to have an unchecked conversion function that doesn't return a Result, so it is faster and more convenient to use. This comes at the cost that the user is responsible to ensure the conversion never panics.

On the other hand, there is a try_from_int function, that returns a result which can be used alternatively.

ethane-abi/src/parameter/mod.rs Outdated Show resolved Hide resolved
ethane-wasm/www/.travis.yml Show resolved Hide resolved
ethane-wasm/www/LICENSE-APACHE Show resolved Hide resolved
ethane-wasm/www/LICENSE-MIT Show resolved Hide resolved
ethane-wasm/www/index.html Show resolved Hide resolved
ethane-wasm/www/index.js Show resolved Hide resolved
ethane-wasm/www/package.json Show resolved Hide resolved
ethane-wasm/www/webpack.config.js Show resolved Hide resolved
ethane-wasm/www/.bin/create-wasm-app.js Show resolved Hide resolved
- Untrack `www` folder
- show decimal equivalent of some hex numbers
@PopcornPaws
Copy link
Member Author

Found a bug:
Ethane-abi Parameter holds a H256 type internally, that is displayed on 64 bytes. However, when the parameter is an Address, it should only display 40 bytes! This affects the to_string() method's result as well.

Mark Melczer added 3 commits May 24, 2021 16:36
`Parameter` stores data in a `H256` type, but it wasn't converted to an
`Address`, when displaying it. This means, that instead of 20 bytes, 32
bytes were displayed.
@PumpkinSeed
Copy link
Member

Decline because the new PR contains these changes as well.

@PumpkinSeed PumpkinSeed deleted the I36-remove-deps branch May 25, 2021 11:22
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.

Sometimes tests fail unexpectedly Decrease dependency count as much as possible
2 participants