Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Native: swap Policy's [get/set]AttributeFee implementations #3859

Merged
merged 4 commits into from
Apr 2, 2025

Conversation

AnnaShaleva
Copy link
Member

Description

Problem:

[get/set]AttributeFee method of native Policy contract has two implementations: pre-Echidna and post-Echidna (ref. #3175). The problem is that two implementations of this method have reversed ActiveIn/ActiveTill meaning.

Solution:

GetAttributeFeeV0 implementaiton of Policy's getAttributeFee method should be enabled till Echidna, whereas GetAttributeFee should be enabled starting from Echidna. The same is relevant for SetAttributeFeeV0 and SetAttributeFeeV1 implementations.

Type of change

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

Problem:

`[get/set]AttributeFee` method of native Policy contract has two
implementations: pre-Echidna and post-Echidna (ref. #3175). The problem
is that two implementaitons of this method have reversed
ActiveIn/ActiveTill meaning.

Solution:

GetAttributeFeeV0 implementaiton of Policy's `getAttributeFee` method
should be enabled *till* Echidna, whereas GetAttributeFee should be
enabled *starting from* Echidna. The same is relevant for
SetAttributeFeeV0 and SetAttributeFeeV1 implementations.

Signed-off-by: Anna Shaleva <shaleva.ann@nspcc.ru>
This renaming allows to avoid bugs and collisions with
ActiveIn/ActiveTill. Also, this renaming follows the setters style (for
setters we have `SetAttributeFeeV0` and `SetAttributeFeeV1`.

No functional changes, just a refactoring. Contract manifest is not affected.

Signed-off-by: Anna Shaleva <shaleva.ann@nspcc.ru>
@AnnaShaleva AnnaShaleva added Bug Used to tag confirmed bugs Waiting for Review labels Mar 31, 2025
@AnnaShaleva AnnaShaleva added this to the v3.8.0 milestone Mar 31, 2025
Copy link
Contributor

@Wi1l-B0t Wi1l-B0t left a comment

Choose a reason for hiding this comment

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

LGTM

@Jim8y
Copy link
Contributor

Jim8y commented Apr 1, 2025

LMAO, that is really easy to be ignored, maybe we should explicityly use deactive = true....

@NGDAdmin NGDAdmin merged commit 82c5859 into master Apr 2, 2025
6 checks passed
@NGDAdmin NGDAdmin deleted the echidna-attr-fee branch April 2, 2025 07:56
AnnaShaleva added a commit that referenced this pull request Apr 3, 2025
No functional changes, just adopt the #3859.

Signed-off-by: Anna Shaleva <shaleva.ann@nspcc.ru>
Jim8y added a commit to Jim8y/neo that referenced this pull request Apr 9, 2025
* master: (163 commits)
  [style] Added some var styles (neo-project#3867)
  [`fix`] stop syncing on block 1465790 (neo-project#3888)
  Optimize block deserialization (neo-project#3879)
  Avoid double `ToArray` on `OnInvMessageReceived` (neo-project#3875)
  style: format long lines (neo-project#3884)
  optimize: return GetFileNameWithoutExtension(Path) if name is not set (neo-project#3883)
  Fix possible null exception (neo-project#3880)
  Remove linkedList (neo-project#3873)
  Optimize Uint160 and Uint256 constructor (neo-project#3872)
  Release the resources (neo-project#3868)
  [Clean] Remove `IRawReadOnlyStore` (neo-project#3869)
  move non-plugins out of plugins (neo-project#3863)
  feature: set name when create wallet (neo-project#3866)
  Native: swap Policy's `[get/set]AttributeFee` implementations (neo-project#3859)
  Fix: concurrent conflict in Cache.CopyTo (neo-project#3860)
  Fix: add default key parameter in help cmd (neo-project#3865)
  [Plugin UT] add more rpcserver UTs (neo-project#3864)
  config: upgrade NeoFS chains protocol configuration (neo-project#3858)
  [`Optimization`]: add exception message to `ArgumentException` (neo-project#3862)
  Native: unify arguments naming of CryptoLib's `verifyWith*` methods (neo-project#3855)
  ...

# Conflicts:
#	benchmarks/Neo.VM.Benchmarks/OpCode/Arrays/OpCode.ReverseN.cs
#	benchmarks/Neo.VM.Benchmarks/Program.cs
#	src/Neo/Neo.csproj
#	src/Neo/ProtocolSettings.cs
#	src/Neo/SmartContract/ApplicationEngine.cs
#	src/Neo/SmartContract/Native/NeoToken.cs
#	src/Neo/SmartContract/Native/RoleManagement.cs
#	tests/Neo.UnitTests/SmartContract/Manifest/UT_ContractManifest.cs
#	tests/Neo.UnitTests/SmartContract/Manifest/UT_ContractPermission.cs
#	tests/Neo.UnitTests/SmartContract/Native/UT_NativeContract.cs
#	tests/Neo.UnitTests/SmartContract/Native/UT_NeoToken.cs
#	tests/Neo.UnitTests/UT_ProtocolSettings.cs
#	tests/Neo.VM.Tests/UT_ReferenceCounter.cs
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants