Strings library

[abcoathup]

WIP for #1745

Add basic Strings library with Concatenate and UintToString functions to start things off.

[nventuro]

Great work @abcoathup, thank you very much! I left a couple comments on both the contracts and the test structure.

[abcoathup]

Outstanding is the inner working of the functions. All the other changes requested should be updated.

You had asked about bytes memory bytesA = bytes(a); whether this creates a new array or is an alias. Unfortunately I don't know

[nventuro]

Awesome @abcoathup, thanks!

This is all I've found regarding copy operations: https://solidity.readthedocs.io/en/v0.5.8/control-structures.html?highlight=copy#complications-for-arrays-and-structs

It's sadly incomplete and not very clear, I've asked in the Solidity gitter channel about this: https://gitter.im/ethereum/solidity?at=5cd9bb7c0f381d0a76add441

[nventuro]

Apparently those assignments won't create actual copies of the data (as you've assumed), but I'm not yet 100% sure that's the case. I created ethereum/solidity#6763 asking for the official documentation to be update, we should be on the lookout for replies to it.

[nventuro]

Thanks for your work @abcoathup! I left a couple comments about the tests and implementation.

[nventuro]

So, after looking around for a bit for the word-copying thing I mentioned, I came across https://github.com/Arachnid/solidity-stringutils, which is rather complete and well-made, albeit unmaintained (it targets Solidity ^0.4.14, and there's been a PR adding 0.5 support open for over two months with no replies). It also lacks distribution and versioning (the readme suggests importing from the GitHub master branch). It's Apache v2 licensed.

@frangio what do you think we should do in this scenario? I wouldn't mind having it as a dependency, if it weren't for the lack of versioning, distribution, and support for v0.5.x. Bringing (a subset) of it into OpenZeppelin also sounds like a good idea (it'd be adding what this very PR adds), but I'm not sure how licensing would play into that.

[frangio]

Having it as a dependency is not a possibility, because smart contract development tools AFAIK don't support "transitive" dependencies.

Bringing some of that project's code into OpenZeppelin would be interesting but I also don't know how licensing plays into it. I would prefer to just implement ourselves the little functionality needed for this PR.

[nventuro]

We could extend our prepack script to bundle those dependencies, as a workaround.

Implementing this ourselves is not a bad plan, but I worry there may be tricky bits we haven't considered (like my comment about UTF-8 strings) that have been already tackled by that library.

[abcoathup]

I mentioned Nick's string library in the issue #1745 as I had only just come across it. Sorry if that wasn't clearer.

Though may want to consider the strings library by Nick Johnson https://github.com/Arachnid/solidity-stringutils/blob/master/src/strings.sol

I have changed the concatenate implementation as I just came across a different solution which looks ok using abi.encodePacked

https://ethereum.stackexchange.com/a/56337/53724
ethereum/solidity#4704

What do you think for concatenation?

For the use case of returning a token URI we only need concatenation and conversion of a uint256 to a string.

[frangio]

Ohh that is very cool! I personally would consider removing concatenate altogether then. abi.encodePacked is superior in that it can receive a variable number of arguments.

[nventuro]

Awesome, thanks a lot for your work @abcoathup!