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

Make use of external functions #569

Open
elenadimitrova opened this issue Mar 15, 2019 · 5 comments
Open

Make use of external functions #569

elenadimitrova opened this issue Mar 15, 2019 · 5 comments

Comments

@elenadimitrova
Copy link
Contributor

External functions can be used to save gas when large arrays of data are being received. This is especially true for the reputation mining process where such large input arrays are accepted. An experimental branch was pushed that started testing this which shows 1,000 gas savings in confirmJustificationRootHash (down from 139140 to 138377).

Note that to enable multi-dimensional array to be accepted in an external function, the ABIEncoderV2 has to be enabled. See documentation note here

Currently we use dynamic size input array of bytes in some external candidate reputation functions which blocks their switch to external, however this will potentially be unblocked when #393 is done.

@area
Copy link
Member

area commented Mar 28, 2019

While doing #393 with an eye on unblocking this issue, I refactored to a point where in principle, we should have been able switch to external on respondToChallenge. However, doing so gave us the dreaded 'stack too deep' error. Elena found this, and it seems logical to assume the same issue applies to bytes32[] which... we have an awful lot of in that function call. The only solution at this juncture would seem to be compressing all of our bytes32[]s in to one giant bytes32[], but we've opted to not do that for intelligibility purposes right now. We're going to be the only entity reputation mining at the start, so shouldn't need to use this function so we can hold off such drastic refactoring action for now.

(Note that we also need to remove the lines

u[U_DECAY_TRANSITION] = 0;
    u[U_GLOBAL_CHILD_UPDATE] = 0;
    u[U_NEW_REPUTATION] = 0;

as calldata is read only. Two of these are fairly clean to the remove, but the third is less so and should warrant more consideration when this is being done).

@dev1644
Copy link
Contributor

dev1644 commented Apr 10, 2019

Does this issue require some work or is it fixed?

@elenadimitrova
Copy link
Contributor Author

As per @area 's comment above it's iceboxed for now.

@dev1644
Copy link
Contributor

dev1644 commented Apr 10, 2019

Are you guys waiting for the ABIEncoderv2 to be released for this issue?

@elenadimitrova
Copy link
Contributor Author

No, we have a "stack too deep" issue, like I said, read the last comment for details.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants