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

[suggestion] Expand Multiply expression to more types #2543

Closed
Arjentix opened this issue Jul 26, 2022 · 7 comments
Closed

[suggestion] Expand Multiply expression to more types #2543

Arjentix opened this issue Jul 26, 2022 · 7 comments
Assignees
Labels
api-changes Changes in the API for client libraries Enhancement New feature or request iroha2-dev The re-implementation of a BFT hyperledger in RUST

Comments

@Arjentix
Copy link
Contributor

Feature request

@appetrosyan suggested here to impl Multiply expression for more than just U32

Motivation

More types Multiply supports -- the more flexible this instruction is

Who can help?

@appetrosyan

@Arjentix Arjentix added Enhancement New feature or request iroha2-dev The re-implementation of a BFT hyperledger in RUST api-changes Changes in the API for client libraries labels Jul 26, 2022
@appetrosyan appetrosyan added the question Further information is requested label Sep 12, 2022
@appetrosyan
Copy link
Contributor

Research how Ethereum does it.

@appetrosyan appetrosyan self-assigned this Sep 12, 2022
@appetrosyan appetrosyan assigned Arjentix and unassigned appetrosyan Sep 14, 2022
@Arjentix
Copy link
Contributor Author

Arjentix commented Sep 14, 2022

That's what the Solidity docs say:

Prior to Solidity 0.8.0, arithmetic operations would always wrap in case of under- or overflow leading to widespread use of libraries that introduce additional checks.

Since Solidity 0.8.0, all arithmetic operations revert on over- and underflow by default, thus making the use of these libraries unnecessary.

Although modern Solidity has a way to bring back unchecked operations we aren't interested in that.

And some docs about revert which was mentioned above:

... They (errors) have to be used together with the revert statement which causes all changes in the current call to be reverted and passes the error data back to the caller

So this is like panicking in our case

@Arjentix
Copy link
Contributor Author

Options we have:

  1. Copy Solidity behaviour
  2. Copy Rust behaviour
  3. Remove Multiply at all

@Arjentix Arjentix assigned appetrosyan and unassigned Arjentix Sep 15, 2022
@appetrosyan
Copy link
Contributor

What's the use case for leaving multiply @Arjentix? To me it seems that we are guaranteed both precision loss and overcomplicate expressions.

@Arjentix
Copy link
Contributor Author

@appetrosyan , in my opinion, we should remove not only Multiply then. We should find a way to diferentiate between things we want to keep and things we want to remove. I don't see such a sign now.

@appetrosyan
Copy link
Contributor

Expand functionality of expression to handle complex arithmetic. Need conversion methods. Must create epic

@appetrosyan appetrosyan removed question Further information is requested research labels Oct 24, 2022
@appetrosyan appetrosyan assigned Erigara and unassigned appetrosyan Nov 3, 2022
@Erigara
Copy link
Contributor

Erigara commented Nov 11, 2022

Closed in favor of #2905.

@Erigara Erigara closed this as completed Nov 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-changes Changes in the API for client libraries Enhancement New feature or request iroha2-dev The re-implementation of a BFT hyperledger in RUST
Projects
None yet
Development

No branches or pull requests

3 participants