Skip to content
This repository was archived by the owner on Mar 19, 2019. It is now read-only.

Conversation

@dong77
Copy link
Contributor

@dong77 dong77 commented Apr 22, 2018

No description provided.

@dong77 dong77 requested a review from kongliangzhong April 22, 2018 01:26
@Brechtpd
Copy link
Contributor

Brechtpd commented Apr 22, 2018

Doing bytesToBytes32 like this will be significantly faster (should reduce gas by about ~7500 for every call). Will make a PR if you want.

function bytesToBytes32(
        bytes b,
        uint offset
        )
        public
        pure
        returns (bytes32 out)
    {
        require(b.length >= offset + 32);
        bytes32 temp;
        assembly {
            temp := mload(add(add(b, 0x20), offset))
        }
        return temp;
    }

@dong77
Copy link
Contributor Author

dong77 commented Apr 22, 2018 via email

@Brechtpd
Copy link
Contributor

See #321

@Hephyrius
Copy link
Contributor

Hephyrius commented Apr 22, 2018

I feel like encoding the data as a bytes array would be a bit clunky and would waste a bit too much memory as an overhead, I do have an idea in mind but I'm unsure if Daniel would like it.

What I propose is:
bytes32[] similar to the BatchTransfer bytes32[]. The very first element of the bytes32[] would be made up of all the V (uint8) values. Each byte of the bytes32 would be mapped to each order in a manner similar to the bit mapping used in feeselection.

Once Tests are working for 1.6 , I will test the idea out and see if it is viable.

Edit:
I will test the idea out on code as is, I think it would be easy to adapt it to 1.6 when that branch is ready. V, R and S are relatively untouched in 1.6 by the looks of it. I will update once I know if this is worthwhile.

@Brechtpd
Copy link
Contributor

@Hephyrius If I'm interpreting the code correctly, the idea seems to be that the code would support multiple ways of signing without changing the API, hence an opaque array of bytes that contains an ID for the algorithm that should be used for the verification. This way any data can be send for the verification as is needed for the chosen algorithm. Using anything else than a simple byte array (or something equivalent) would limit the extensibility.

@Hephyrius
Copy link
Contributor

Hephyrius commented Apr 22, 2018

This is where the suggestion of the bytes32 being used as the V values could be explored further, say having the first byte of the first bytes32 item be the algorithm selected followed by the non-32byte encoded data for that sig, I think a bytes array is way too expensive and wasteful for most use cases, especially when the bytes array will itse;f form part of another array.

@dong77
Copy link
Contributor Author

dong77 commented Apr 22, 2018 via email

@dong77
Copy link
Contributor Author

dong77 commented Apr 22, 2018 via email

@Brechtpd
Copy link
Contributor

That's correct Daniel. From the solidity documentation:

Variables of type bytes and string are special arrays. A bytes is similar to byte[], but it is packed tightly in calldata. string is equal to bytes but does not allow length or index access (for now).

So bytes should always be preferred over byte[] because it is cheaper.

@dong77 dong77 merged commit 806e407 into version1.6 Apr 23, 2018
@dong77 dong77 deleted the v1.6-multihash branch April 23, 2018 04:34
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants