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

Add "fromAddress" to utils/Strings.sol #2197

Closed
BlinkyStitt opened this issue Apr 17, 2020 · 12 comments
Closed

Add "fromAddress" to utils/Strings.sol #2197

BlinkyStitt opened this issue Apr 17, 2020 · 12 comments
Labels
feature New contracts, functions, or helpers.

Comments

@BlinkyStitt
Copy link

🧐 Motivation

I am trying to get more useful revert messages in my contracts and being able to format uint256 as a string has been really helpful. It would be nice if the other types had this, too. For now, I only need fromAddress.

📝 Details

There are a couple (slightly outdated) implementations here: https://ethereum.stackexchange.com/questions/8346/convert-address-to-string

@BlinkyStitt
Copy link
Author

BlinkyStitt commented Apr 17, 2020

I tried all the methods on that stackexchange link and only one (by k06a and Aliaksandr Adzinets) worked for me:

    function toString(address x) internal pure returns (string memory) {
        bytes32 value = bytes32(uint256(x));
        bytes memory alphabet = "0123456789abcdef";

        bytes memory str = new bytes(42);
        str[0] = '0';
        str[1] = 'x';
        for (uint i = 0; i < 20; i++) {
            str[2+i*2] = alphabet[uint(uint8(value[i + 12] >> 4))];
            str[3+i*2] = alphabet[uint(uint8(value[i + 12] & 0x0f))];
        }
        return string(str);
    }

I don't know how efficient this is gas-wise, but that doesn't matter for my current purposes.

@abcoathup
Copy link
Contributor

Hi @wysenynja! Thanks for the suggestion, it is really appreciated.

The project owner should review your suggestion during the next week.

Please wait until we have discussed this idea before writing any code or submitting a Pull Request, so we can go through the design beforehand. We don’t want you to waste your time!

@abcoathup abcoathup added the feature New contracts, functions, or helpers. label Apr 21, 2020
@BlinkyStitt
Copy link
Author

You're welcome, and thank you! I'm using that code for now since it works. I'll let your team improve the design.

@nventuro
Copy link
Contributor

Could you share a short snippet of how you're using this to create nicer error messages?

I'd rather see tooling move in a direction where we can retrieve values directly without having to convert them to strings inside the EVM (!), though I guess there's not much harm in having such a helper. I wonder how easy it'd be to create these messages though.

@BlinkyStitt
Copy link
Author

BlinkyStitt commented Apr 24, 2020

This is what I'm doing for now. I just wanted useful reverts in dev so I'm not caring about gas efficiency at all.

if (ending_vault_balance < starting_vault_balance) {
  uint256 decreased_amount = starting_vault_balance - ending_vault_balance;
  string memory err = string(abi.encodePacked("Vault balance of ", address(borrow_token).toString(), " did not increase. Decreased by ", decreased_amount.toString()));
  revert(err);
}

This one mostly works, but theres some bytes at the front of call_returned that I don't know how to deal with. That's a separate issue though.

(bool success, bytes memory call_returned) = action_address.call{value: action_value}(actions[i].data);

if (!success) {
    string memory err = string(abi.encodePacked("on call #", i.toString()," to ", action_address.toString(), " with ", action_value.toString(), " ETH failed: '", string(call_returned), "'"));
    revert(err);
}

@nventuro
Copy link
Contributor

nventuro commented May 6, 2020

Hello @wysenynja, sorry to have left you hanging.

Even if usefulness of this feature is unclear, we think it might make sense to include it in the OpenZeppelin Contracts library to provide users with the flexibility to perform these sort of tasks. We arrived at a similar conclusion in #1795 (which is rather relevant here).

Unfortunately, we don't have enough bandwidth to properly include features that will likely go unsed, with the associated tests, documentation and maintenance tasks, and think our effors will have a larger impact in areas where there's larger evidence of community demand. Head to our roadmap to learn about what we're currently working on.

I'll keep this issue open as a space for people to discuss this and related features - if we find interesting use cases where such as thing is needed, then we can carry on work and think how to best support them from the Contracts library.

Thank you very much for your work and suggestions @wysenynja!

@BlinkyStitt
Copy link
Author

BlinkyStitt commented May 7, 2020

That makes sense. For now, I have a Strings2.sol that is simple enough to add the same way that I add OpenZepplin's Strings.sol

pragma solidity 0.6.7;

library Strings2 {
    function toString(address x) internal pure returns (string memory) {
        bytes32 value = bytes32(uint256(x));
        bytes memory alphabet = "0123456789abcdef";

        bytes memory str = new bytes(42);
        str[0] = '0';
        str[1] = 'x';
        for (uint i = 0; i < 20; i++) {
            str[2+i*2] = alphabet[uint(uint8(value[i + 12] >> 4))];
            str[3+i*2] = alphabet[uint(uint8(value[i + 12] & 0x0f))];
        }
        return string(str);
    }
}

@BlinkyStitt
Copy link
Author

Oops. I mis-clicked.

@ItsNickBarry
Copy link

I have a situation where n contracts may be deployed and linked to a corresponding ERC20 contract. Users may hold tokens from any number of these ERC20 contracts.

The token metadata is generated using the string representation of the address of the linked contract. Something like string(abi.encodePacked('Linked: ', address(_linkedContract).toString())).

This isn't the ideal UI feature, but compare it to Uniswap's tokens, for example, each of which is nondescriptly named "Uniswap V1 (UNI-V1)".

@nventuro
Copy link
Contributor

Thanks for sharing your use case @ItsNickBarry! That's very interesting.

A slight issue with this is that the addresses wouldn't be checksummed. It doesn't seem like it'd be terribly hard to implement the checksum, though we'd have to hash the address and access its bits, requiring some masking.

@Amxx
Copy link
Collaborator

Amxx commented Nov 29, 2021

This is resolved by this function.

string fromAddress = toHexString(uint256(uint160(myAddress), 20)

@Amxx Amxx closed this as completed Nov 29, 2021
@frangio
Copy link
Contributor

frangio commented Dec 1, 2021

@Amxx What do you think about adding an override for toHexString so a user could simply write toHexString(myAddress)?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New contracts, functions, or helpers.
Projects
None yet
Development

No branches or pull requests

6 participants