Split NEP-141 logic #607
Open
Split NEP-141 logic #607
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…o feat/split-eth-connector
…o feat/split-eth-connector
# Conflicts: # Cargo.lock # engine-standalone-storage/src/relayer_db/mod.rs # engine-standalone-storage/src/sync/mod.rs # engine-standalone-storage/src/sync/types.rs # engine-tests/src/tests/eth_connector.rs # engine-tests/src/tests/state_migration.rs # engine-tests/src/tests/xcc.rs # engine-types/src/storage.rs # engine-types/src/types/balance.rs # engine/src/admin_controlled.rs # engine/src/connector.rs # engine/src/lib.rs # engine/src/parameters.rs
aleksuss
reviewed
Mar 14, 2023
engine/src/metadata.rs
Outdated
There was a problem hiding this comment.
metadata is a more common name. I would use something more specific for that. E.g. ft_metadata?
There was a problem hiding this comment.
Actually, it shouldn't even exist. But before removing NEP-141, I think it's more efficient to keep it in Engine because it returns just const data. If we change it to a cross-contract call, we should attach gas. So really temporary solution.
There was a problem hiding this comment.
Why remove some API but then keep others? It isn't the nETH contract anymore.
Co-authored-by: Oleksandr Anyshchenko <oleksandr.anyshchenko@aurora.dev>
Co-authored-by: Oleksandr Anyshchenko <oleksandr.anyshchenko@aurora.dev>
olga24912
reviewed
May 11, 2023
| method: "engine_ft_transfer_call".to_string(), | ||
| args: data, | ||
| attached_balance: Yocto::new(1), | ||
| attached_gas: GAS_FOR_FT_TRANSFER_CALL, |
There was a problem hiding this comment.
It is unpredictable how much gas the ft_transfer_call cost because it depends on the ft_on_transfer function. But here we provide the constant gas amount and users don't have the opportunity to increase it.
What about using the trick from the standard realization: https://docs.rs/near-contract-standards/latest/src/near_contract_standards/fungible_token/core_impl.rs.html#146 ?
engine/src/connector.rs
Outdated
| method: "ft_metadata".to_string(), | ||
| args: Vec::new(), | ||
| attached_balance: ZERO_ATTACHED_BALANCE, | ||
| attached_gas: GAS_FOR_FINISH_DEPOSIT, |
engine/src/connector.rs
Outdated
| method: "get_bridge_prover".to_string(), | ||
| args: Vec::new(), | ||
| attached_balance: ZERO_ATTACHED_BALANCE, | ||
| attached_gas: GAS_FOR_FINISH_DEPOSIT, |
engine/src/connector.rs
Outdated
| method: "get_accounts_counter".to_string(), | ||
| args: Vec::new(), | ||
| attached_balance: ZERO_ATTACHED_BALANCE, | ||
| attached_gas: GAS_FOR_FINISH_DEPOSIT, |
# Conflicts: # .github/workflows/tests.yml # Cargo.lock # Cargo.toml # Makefile.toml # engine-standalone-nep141-legacy/src/fungible_token.rs # engine-standalone-storage/Cargo.toml # engine-standalone-storage/src/sync/mod.rs # engine-tests/Cargo.toml # engine-tests/src/tests/erc20_connector.rs # engine-tests/src/tests/eth_connector.rs # engine-tests/src/tests/mod.rs # engine-tests/src/tests/repro.rs # engine-tests/src/tests/sanity.rs # engine-tests/src/tests/state_migration.rs # engine-tests/src/tests/uniswap.rs # engine-tests/src/tests/xcc.rs # engine-tests/src/utils/mod.rs # engine-tests/src/utils/standalone/mocks/mod.rs # engine-types/src/parameters/mod.rs # engine/src/connector.rs # engine/src/lib.rs # engine/src/parameters.rs
aleksuss
force-pushed
the
feat/split-eth-connector
branch
from
fd29e16
to
5b14000
Compare
aleksuss
force-pushed
the
feat/split-eth-connector
branch
from
5b14000
to
d3eec6f
Compare
joshuajbouw
pushed a commit
that referenced
this pull request
Aug 7, 2023
## Description The PR is a clone of #607 but for SILO smart contract. --------- Co-authored-by: Evgeny Ukhanov <evgeny@aurora.dev> 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
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Split NEP-141
It is based on AIP: Split NEP-141 logic outside of Engine.
The main purpose is to isolate the NEP-141 Fungible Token logic into a separate contract and remove this logic from the Engine.
Aurora Eth Connector contract: https://github.com/aurora-is-near/aurora-eth-connector
Main goals
aurora-eth-connectorcontracteth-connectortestsaurora-eth-connectorSolution
fungible_tokenmoduleaurora-eth-connector.ft_transferft_transfer_callft_total_supplyft_balance_offt_metadatadepositwithdrawstorage_depositstorage_unregisterstorage_balance_offt_on_transferis part of Engine, and responsible as call-back function forft_transfer_callonaurora-eth-connectorconnectormoduleengine-tests-connector, with deploying newaurora-eth-connectorcontractengine-testsall tests related toeth-connectorfinish_depositand other eth-connector specific functions now are part ofaurora-eth-connector. This means that now we cannot receive events from there and process them since these functions are private.standalone-legacy-nep141implementation (Feat: standalone nep141 legacy #669)Gas costs
Not changed.
Breaking changes
The important change is that before the NEP-141 function call was direct, now it is a cross-contract call. This means that in the NEAR blockchain, this challenge is found as a recipe. This means that if the function had previously returned an error, the transaction was rolled back. Now the transaction will be successful if an error occurs in the receipt. This nuance must be taken into account.
CI flow
Github CI flow was changed. In particular, now the current version of
aurora-eth-connectoris downloaded from the remote repository and a contract for tests is being assembled. The run-tests parameters have also been changed, and the number of threads that can be started is limited, which is caused by current restrictionsworkspace.rsissue 253.