feat(runtime): restrict creation of Ethereum address on NEAR #9365
Conversation
bowenwang1996
force-pushed
the
restrict-ethereum-address
branch
from
6a1ec44
to
b6230e7
Compare
bowenwang1996
force-pushed
the
restrict-ethereum-address
branch
2 times, most recently
from
d0d8d4a
to
280689e
Compare
bowenwang1996
force-pushed
the
restrict-ethereum-address
branch
from
280689e
to
b6475c4
Compare
|
The associated NEP near/NEPs#492 has been approved and this implementation is ready for review |
| @@ -120,6 +120,8 @@ pub enum ProtocolFeature { | |||
| RejectBlocksWithOutdatedProtocolVersions, | |||
| #[cfg(feature = "protocol_feature_simple_nightshade_v2")] | |||
| SimpleNightshadeV2, | |||
| #[cfg(feature = "protocol_feature_restrict_tla")] | |||
| RestrictTLA, | |||
There was a problem hiding this comment.
To be consistent with our codebase, please capitalize only the first letter of acronyms. NearVmRuntime is an example of this. This is also in line with Rust RFC 430.
| RestrictTLA, | |
| RestrictTla, |
| let old_condition_for_error = account_id.len() | ||
| < account_creation_config.min_allowed_top_level_account_length as usize | ||
| && predecessor_id != &account_creation_config.registrar_account_id; | ||
| let new_condition_for_error = !account_id.is_implicit() | ||
| && predecessor_id != &account_creation_config.registrar_account_id; | ||
| let is_error = if checked_feature!( | ||
| "protocol_feature_restrict_tla", | ||
| RestrictTLA, | ||
| current_protocol_version | ||
| ) { | ||
| new_condition_for_error | ||
| } else { | ||
| old_condition_for_error | ||
| }; |
There was a problem hiding this comment.
What happens to the now obsolete parameter min_allowed_top_level_account_length: 32?
Have you considered to instead just set min_allowed_top_level_account_length: 65 instead of this code change? Something like 139.yaml:
# Implements NEP-492, disallowing all top-level accounts.
min_allowed_top_level_account_length: { old: 32, new: 65 }I think doing it this way would be less confusing for future development, and it also gives private shards the freedom to overwrite this setting for their purposes.
There was a problem hiding this comment.
Please let us know if you have an opinion on top-level account creation. If Calimero relies on top-level account creation by non-registrar accounts, it would be crucial to know before we stabilize this.
There was a problem hiding this comment.
What happens to the now obsolete parameter
min_allowed_top_level_account_length: 32? Have you considered to instead just setmin_allowed_top_level_account_length: 65instead of this code change? Something like139.yaml:# Implements NEP-492, disallowing all top-level accounts. min_allowed_top_level_account_length: { old: 32, new: 65 }I think doing it this way would be less confusing for future development, and it also gives private shards the freedom to overwrite this setting for their purposes.
yeah this is a very good point. I should stop writing code :)
Proposal to restrict the creation of Ethereum addresses on NEAR to prevent loss of funds and enable future possibilities for Ethereum wallets to support NEAR transactions. Implementation: near/nearcore#9365
Implements near/NEPs#492