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

abi string type, fixes #216, #218, #219 #228

Merged
merged 17 commits into from
Jun 25, 2015
Merged

abi string type, fixes #216, #218, #219 #228

merged 17 commits into from
Jun 25, 2015

Conversation

debris
Copy link
Contributor

@debris debris commented Jun 1, 2015

This pr brings back string type for solidity.

@ethers @chriseth @frozeman can you review tests?

@coveralls
Copy link

Coverage Status

Coverage increased (+0.05%) to 94.53% when pulling fa3239f on strings into 7753724 on develop.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.05%) to 94.53% when pulling 73cc711 on strings into 448dd30 on develop.

@chriseth
Copy link
Contributor

chriseth commented Jun 2, 2015

Would be great to have tests for actual unicode / utf8.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.05%) to 94.53% when pulling 189484f on strings into 448dd30 on develop.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.06%) to 94.54% when pulling 7b730b1 on strings into 448dd30 on develop.

@chriseth
Copy link
Contributor

chriseth commented Jun 8, 2015

Would be great to have tests for en- and decoding the following:

  • correctly encoded utf-8 string containing non-ascii characters
  • incorrectly encoded utf-8 string terminating prematurely (e.g. "\xc3")
  • utf-8 strings containing internal zeros (e.g. "\xc3\xa4\x00\x00\xc3\xa4")

@LefterisJP
Copy link
Contributor

May I suggest this UTF8 encoding/decoding stress testing collection? I use it in one of my projects.

@frozeman
Copy link
Contributor

@debris please merge this PR asp, as solidity has now string and array support and the wallet dapp depends on that and its now not aligned with solidity.

Additionally we should give a note to the community that auto conversion of bytes has been removed.

@frozeman
Copy link
Contributor

@debris URF8 encoding seems to work fine, but decoding seems broken:

> "\xc3\xa4\x00\x00\xc3\xa4"
"ä��ä"

> web3.fromAscii("\xc3\xa4\x00\x00\xc3\xa4")
"0xc3a40000c3a4"

> web3.toAscii('0xc3a40000c3a4')
"ä"

@frozeman
Copy link
Contributor

@debris ok changed, could please also take a look and as well at the encoding issue i mentioned above above:

> "\xc3\xa4\x00\x00\xc3\xa4"
"ää"

> web3.fromAscii("\xc3\xa4\x00\x00\xc3\xa4")
"0xc3a40000c3a4"

> web3.toAscii('0xc3a40000c3a4')
"ä"

Conflicts:
	dist/web3-light.js
	dist/web3-light.min.js
	dist/web3.js
	dist/web3.min.js
…o strings

Conflicts:
	dist/web3-light.js
	dist/web3-light.js.map
	dist/web3-light.min.js
	dist/web3.js
	dist/web3.js.map
	dist/web3.min.js
@debris
Copy link
Contributor Author

debris commented Jun 25, 2015

@debris ok changed, could please also take a look and as well at the encoding issue i mentioned above above:

"\xc3\xa4\x00\x00\xc3\xa4"
"ää"

web3.fromAscii("\xc3\xa4\x00\x00\xc3\xa4")
"0xc3a40000c3a4"

web3.toAscii('0xc3a40000c3a4')
"ä"

decoding solidity strings does not use web3.toAscii internally. I added few tests in 93c8006 . I will fix it in another pr.

Let's merge!

@frozeman
Copy link
Contributor

Ok but then lets actually rename toAscii to toString, as its no converting to ascii but latin something..

frozeman added a commit that referenced this pull request Jun 25, 2015
@frozeman frozeman merged commit 4a46c73 into develop Jun 25, 2015
@frozeman frozeman deleted the strings branch July 7, 2015 08:58
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.

5 participants