Connector cleanup #374

mrLSD · 2021-11-22T16:20:48Z

Cleanup Eth-connector and improve code quality.

Full description here: #359

…call. Extendede comments

…t & related event data

…ransfer, parse_on_transfer_message

birchmd

Thanks for your work on this, it's certainly moving in the right direction!

My main concern is about changing the binary format in one of the messages. That kind of backwards-incompatible change could cause an issue in the future. Remember that we want to support replaying transactions in the standalone engine now, so we need to be cognizant of breaking changes.

engine-tests/src/tests/eth_connector.rs

engine-types/src/types.rs

engine/src/connector.rs

engine/src/deposit_event.rs

engine/src/fungible_token.rs

engine/src/parameters.rs

engine/src/connector.rs

mrLSD · 2021-11-30T13:22:06Z

My main concern is about changing the binary format in one of the messages.

From that point of view, it makes sense. I'll return to the old binary format.

An interesting fact, out of 32 bytes - 16 bytes are superfluous. since we don't use them.

…nged to unwrap

birchmd

I think this PR is nearly ready! In this round of comments I want you delete some of the implementations on the Fee new type because they do not properly follow the semantics of its arithmetic.

engine-tests/src/tests/eth_connector.rs

engine-types/src/types.rs

engine/src/connector.rs

engine/src/engine.rs

engine/src/fungible_token.rs

change comemnts Co-authored-by: Michael Birch <birchmd8@gmail.com>

…rora-engine into feat/connector-cleanup

birchmd

Thanks @mrLSD nice job.

engine/src/engine.rs

…o feat/connector-cleanup

joshuajbouw

Looks good to me. In the future, please do changes in different PRs. Makes it much easier for the reviewers to go through.

engine-types/src/types.rs

engine-tests/src/tests/eth_connector.rs

engine-types/src/types.rs

engine/src/connector.rs

* Feat(engine): London hard fork support (#244) * Fix(exit precompile): Address to refund in case of error is an argument (#311) * Feat(engine): Make engine parametric in storage access (#314) * Test verifying the EVM log returns the correct address (#341) * Remove sdk::current_account_id usage from engine-precompiles (#346) * Remove Default trait bound from engine IO (#342) * Remove some sdk usage from core logic (#347) * Factor out blockchain environment variable access as a trait (#349) * Factor out NEAR promise host functions into a trait (#353) * Borsh deserialized value field for call args (#351) * Refactor(eth-connector): Use Result return values instead of panicking (#355) * Gate all NEAR host functions behind the contract feature (#356) * Bump @openzeppelin/contracts from 4.3.2 to 4.3.3 in /etc/eth-contracts * Chore: Newtypes for gas (#344) * Feat(standalone): Standalone (#345) * Minor fixes to sdk refactor (#359) * Refactor(engine): Move submit logic into engine module (#366) * Feat(standalone): Storage backend (#375) * NEAR random numbers from solidity contract (#368) * Feat(standalone): EVM tracing via SputnikVM (#383) * Feat(standalone): Bootstrap storage from relayer and state snapshots (#379) * Feat(standalone): Structures and logic for keeping storage in sync with the blockchain (#382) * Feat(standalone): Capture geth-like tracing from SputnikVM events (#384) * Connector cleanup (#374) * Remove betanet * Fix(engine): original_storage bug fix; more tracing tests (#390) * Increase NEAR Gas for ft_on_transfer (#389) Co-authored-by: Andrew Bednoff <andrew.bednoff@aurora.dev> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Evgeny Ukhanov <evgeny.ukhanov@aurora.dev> Co-authored-by: Marcelo Fornet <marcelo.fornet@aurora.dev> Co-authored-by: Michael Birch <michael.birch@aurora.dev>

mrLSD added the A-benchmark Area: performance benchmarks label Nov 22, 2021

mrLSD self-assigned this Nov 22, 2021

mrLSD changed the base branch from master to develop November 22, 2021 16:21

mrLSD added 11 commits November 26, 2021 14:59

Added Eth-connector comments and renamde some constants

6d27234

Changed eth-connector contract init instance and comments

88d6ae2

Analized code and added TODO's for next changes. Changes for Trasfer …

c5e3d7f

…call. Extendede comments

Fix ft_transaction

5b7e98e

Introduce Fee type

d8d2038

Added Fee type, and change everywhere. Added changes for deposit_even…

ee9c2c2

…t & related event data

Fix all Fee type dependency. Modified logic: prepare_message_for_on_t…

4013225

…ransfer, parse_on_transfer_message

Fix eth-connecotr tests

fb6d780

Changes for Balance type

2894a28

Cleanup imports

61e0743

Fix standalone/mocks tests

c87c17b

mrLSD force-pushed the feat/connector-cleanup branch from d94ad69 to c87c17b Compare November 26, 2021 13:04

mrLSD added 3 commits November 26, 2021 15:57

Added earlt verification to deposit_event for TokenMessageData

8ba55ed

Extend comments. FungibleToken - cleanup and introduce new type

cb89024

Reorgonize TODO's

819a541

mrLSD marked this pull request as ready for review November 26, 2021 15:51

mrLSD requested a review from joshuajbouw as a code owner November 26, 2021 15:51

mrLSD requested a review from birchmd November 26, 2021 15:51

mrLSD added A-testing Area: If something has added tests, or changed them. C-enhancement Category: New feature or request and removed A-benchmark Area: performance benchmarks labels Nov 26, 2021

birchmd requested changes Nov 29, 2021

View reviewed changes

mrLSD added 3 commits November 30, 2021 17:00

Changed functions: get_recipient, init_contract

7f3266d

deposit_event - check u128 overflow

3056329

parse OnTrasfer: changed Fee to 32 bytes

29326b0

mrLSD requested a review from birchmd November 30, 2021 17:11

mrLSD added 6 commits December 1, 2021 16:56

Fix test_deposit_with_0x_prefix

3fffee6

deposit_event refactoring

ca790c5

Rename OnTransferMessageData -> FtTransferMessageData

5aec1e5

Wei types - added try_into_u128

7e32715

Extend Result types for try_into_u128

a6425b4

Remove redundant Result types for Engine and try_into_u128 - just cha…

fb85325

…nged to unwrap

birchmd requested changes Dec 3, 2021

View reviewed changes

mrLSD and others added 5 commits December 3, 2021 19:17

Simplify BalanceOverflowError, remove From<Fee> for u128

def83bf

Update engine/src/connector.rs

7361b93

change comemnts Co-authored-by: Michael Birch <birchmd8@gmail.com>

Update engine/src/fungible_token.rs

cf997bf

change comemnts Co-authored-by: Michael Birch <birchmd8@gmail.com>

Remove redundant Add<Fee> u128 implementations

a31ec8f

Merge branch 'feat/connector-cleanup' of github.com:aurora-is-near/au…

7a0f163

…rora-engine into feat/connector-cleanup

mrLSD requested a review from birchmd December 3, 2021 17:42

birchmd approved these changes Dec 3, 2021

View reviewed changes

engine/src/engine.rs Show resolved Hide resolved

Merge branch 'develop' of github.com:aurora-is-near/aurora-engine int…

721dcf2

…o feat/connector-cleanup

joshuajbouw approved these changes Dec 8, 2021

View reviewed changes

mrLSD merged commit d05cdd9 into develop Dec 8, 2021

mrLSD deleted the feat/connector-cleanup branch December 8, 2021 18:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Connector cleanup #374

Connector cleanup #374

mrLSD commented Nov 22, 2021

birchmd left a comment

mrLSD commented Nov 30, 2021

birchmd left a comment

birchmd left a comment

joshuajbouw left a comment

Connector cleanup #374

Connector cleanup #374

Conversation

mrLSD commented Nov 22, 2021

birchmd left a comment

Choose a reason for hiding this comment

mrLSD commented Nov 30, 2021

birchmd left a comment

Choose a reason for hiding this comment

birchmd left a comment

Choose a reason for hiding this comment

joshuajbouw left a comment

Choose a reason for hiding this comment