Skip to content

Fix(precompiles): Match on full address when dispatching on precompiles#107

Merged
artob merged 2 commits intodevelopfrom
82-precompile-addresses
May 28, 2021
Merged

Fix(precompiles): Match on full address when dispatching on precompiles#107
artob merged 2 commits intodevelopfrom
82-precompile-addresses

Conversation

@birchmd
Copy link
Copy Markdown
Member

@birchmd birchmd commented May 27, 2021

Fixes #82

@birchmd birchmd added C-bug Category: Something isn't working. C-enhancement Category: New feature or request labels May 27, 2021
@birchmd birchmd requested review from artob, joshuajbouw and mfornet May 27, 2021 16:15
@birchmd birchmd linked an issue May 27, 2021 that may be closed by this pull request
Potential collision with precompiles in address space #82
Closed
sept-en
sept-en approved these changes May 27, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@sept-en sept-en left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work! I really like this approach for handling addresses, very neat!

Added a minor comment regarding tests. Though if we are in hurry to merge it, feel free to ignore.

Comment thread src/precompiles/mod.rs
birchmd
birchmd commented May 27, 2021
View reviewed changes
Comment thread src/precompiles/mod.rs
artob
artob approved these changes May 28, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@artob artob left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

An interesting approach; I'd love to know, though, how Rust compiles those match expressions: do they just turn into a sequence of memcmp branches?

@artob artob self-assigned this May 28, 2021
@artob artob merged commit 046a560 into develop May 28, 2021
@artob artob deleted the 82-precompile-addresses branch May 28, 2021 14:26
@birchmd
Copy link
Copy Markdown
Member Author

birchmd commented May 28, 2021

I wasn't totally sure how it would compile down, so I checked in Compile Explorer.

The result is here: https://gist.github.com/birchmd/44ffe28000662fd1f1f0abe7c528c76c

Since the byte arrays are known at compile time, it looks like the byte comparisons get hard coded into the instructions themselves.

sept-en added a commit that referenced this pull request May 28, 2021
@sept-en @artob
@birchmd @matklad
Eth-connector: merge develop branch (#109)
540ccb1 
* Document `aurora encode-address` usage.

* Cache cargo artifacts between CI runs. (#92)

* Address comments from audit. (#86)

* Validate register length in `read_input_arr20()`
* Only read register length in `Engine::get_code_size`
* Add `read_input_borsh()`
* Ensure `method.args.len() == args_decoded.len()`
* Ensure register size is 8 in `read_u64`
* Use constant to specify the register ID used in `read_input()`

* Reduce size of `cargo cache` in CI. (#95)

* Define a `Wei` newtype for balances. (#96)

* Fix evm-bully builds after recent refactoring. (#100)

* Refactor `Engine::get_state` to return a `Result`. (#99)

* Ensure that `Cargo.lock` in the repo is valid. (#101)

* Remove unneeded nightly feature. (#102)

* Dispatch precompiles on the full address. (#107)

* Support state migration on upgrade. (#103)

* Remove resolved TODOs

* Fix state migration test

* Conditional compilation minor improvements.

Co-authored-by: Arto Bendiken <arto@aurora.dev>
Co-authored-by: Michael Birch <michael@near.org>
Co-authored-by: Aleksey Kladov <aleksey.kladov@gmail.com>
@artob
Copy link
Copy Markdown
Contributor

artob commented May 28, 2021

I wasn't totally sure how it would compile down, so I checked in Compile Explorer.

Thanks for that. Here's the corresponding WebAssembly output: https://godbolt.org/z/s3vK4zPdn

That's about 5,400 WebAssembly instructions, with 20 distinct blocks, for a match expression containing 9 branches. That probably compiles to about 10+ KB in the binary.

@birchmd birchmd mentioned this pull request May 31, 2021
Closed
mfornet added a commit that referenced this pull request Jun 4, 2021
@joshuajbouw @mfornet
@artob @birchmd @matklad @mrLSD
Implement generational storage (#87)
d67b59e 
* Base precompile code between connectors (#73)

* Base precompile code between connectors

* Handle errors and validate input

* Use proper result

* Document `aurora encode-address` usage.

* Cache cargo artifacts between CI runs. (#92)

* Address comments from audit. (#86)

* Validate register length in `read_input_arr20()`
* Only read register length in `Engine::get_code_size`
* Add `read_input_borsh()`
* Ensure `method.args.len() == args_decoded.len()`
* Ensure register size is 8 in `read_u64`
* Use constant to specify the register ID used in `read_input()`

* Reduce size of `cargo cache` in CI. (#95)

* Define a `Wei` newtype for balances. (#96)

* Fix evm-bully builds after recent refactoring. (#100)

* Refactor `Engine::get_state` to return a `Result`. (#99)

* Ensure that `Cargo.lock` in the repo is valid. (#101)

* Remove unneeded nightly feature. (#102)

* Implement BC generational storage.

* fix address input

* remove note

* put key on the end of the storage key

* remove pub from methods

* Dispatch precompiles on the full address. (#107)

* Support state migration on upgrade. (#103)

* Implement the ETH connector. (#59)

* Move when to call `set_generation`

* Fix arg

Co-authored-by: Marcelo Fornet <mfornet94@gmail.com>
Co-authored-by: Arto Bendiken <arto@aurora.dev>
Co-authored-by: Michael Birch <michael@near.org>
Co-authored-by: Aleksey Kladov <aleksey.kladov@gmail.com>
Co-authored-by: Evgeny Ukhanov <mrlsd@ya.ru>
@birchmd birchmd mentioned this pull request Jun 9, 2021
Merged
mfornet added a commit that referenced this pull request Jun 10, 2021
@mfornet @birchmd
@joshuajbouw @artob @matklad @mrLSD
Implement generational storage with SELFDESTRUCT tests (#84)
2bf9b25 
* Add tests for state check after selfdestruct

* Aurora runner tracks storage usage to avoid underflow when storage is released in future transactions (#85)

* Implement generational storage (#87)

* Base precompile code between connectors (#73)

* Base precompile code between connectors

* Handle errors and validate input

* Use proper result

* Document `aurora encode-address` usage.

* Cache cargo artifacts between CI runs. (#92)

* Address comments from audit. (#86)

* Validate register length in `read_input_arr20()`
* Only read register length in `Engine::get_code_size`
* Add `read_input_borsh()`
* Ensure `method.args.len() == args_decoded.len()`
* Ensure register size is 8 in `read_u64`
* Use constant to specify the register ID used in `read_input()`

* Reduce size of `cargo cache` in CI. (#95)

* Define a `Wei` newtype for balances. (#96)

* Fix evm-bully builds after recent refactoring. (#100)

* Refactor `Engine::get_state` to return a `Result`. (#99)

* Ensure that `Cargo.lock` in the repo is valid. (#101)

* Remove unneeded nightly feature. (#102)

* Implement BC generational storage.

* fix address input

* remove note

* put key on the end of the storage key

* remove pub from methods

* Dispatch precompiles on the full address. (#107)

* Support state migration on upgrade. (#103)

* Implement the ETH connector. (#59)

* Move when to call `set_generation`

* Fix arg

Co-authored-by: Marcelo Fornet <mfornet94@gmail.com>
Co-authored-by: Arto Bendiken <arto@aurora.dev>
Co-authored-by: Michael Birch <michael@near.org>
Co-authored-by: Aleksey Kladov <aleksey.kladov@gmail.com>
Co-authored-by: Evgeny Ukhanov <mrlsd@ya.ru>

* Fix layout of the key

* Fix all tests (don't wipe the storage all the time)

* Use correct generation in writing storage

* Remove unnecessary references

Co-authored-by: Michael Birch <michael@near.org>
Co-authored-by: Joshua J. Bouw <dev@joshuajbouw.com>
Co-authored-by: Arto Bendiken <arto@aurora.dev>
Co-authored-by: Aleksey Kladov <aleksey.kladov@gmail.com>
Co-authored-by: Evgeny Ukhanov <mrlsd@ya.ru>
Co-authored-by: Michael Birch <michael.birch@aurora.dev>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@joshuajbouw joshuajbouw Awaiting requested review from joshuajbouw

@mfornet mfornet Awaiting requested review from mfornet

2 more reviewers

@sept-en sept-en sept-en approved these changes

@artob artob artob approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@artob artob

Labels

C-bug Category: Something isn't working. C-enhancement Category: New feature or request

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Potential collision with precompiles in address space

3 participants

@birchmd @artob @sept-en