Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Adding keccak256 #2925

Merged
merged 40 commits into from
Jan 11, 2024
Merged

Conversation

Jim8y
Copy link
Contributor

@Jim8y Jim8y commented Oct 2, 2023

No description provided.

@Jim8y
Copy link
Contributor Author

Jim8y commented Oct 2, 2023

mistakenly deleted previous branch

Copy link
Member

@vncoelho vncoelho left a comment

Choose a reason for hiding this comment

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

@Liaojinghui ,maybe we need more test cases on edge cases.
It would be good to add more.

Furthermore, what is the idea for future resync and forks? @vang1ong7ang

shargon
shargon previously approved these changes Oct 4, 2023
Copy link
Member

@vncoelho vncoelho left a comment

Choose a reason for hiding this comment

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

Decide about the possible attack as on 3.6.0 upgrade, and also verify more tests.

@Jim8y
Copy link
Contributor Author

Jim8y commented Oct 9, 2023

Decide about the possible attack as on 3.6.0 upgrade, and also verify more tests.

Hi vitor, what this means?

@Jim8y
Copy link
Contributor Author

Jim8y commented Oct 9, 2023

@shargon Tests ok and ready for merge.

@shargon
Copy link
Member

shargon commented Oct 9, 2023

Decide about the possible attack as on 3.6.0 upgrade, and also verify more tests.

@vncoelho what this means?

@vncoelho
Copy link
Member

vncoelho commented Oct 9, 2023

Nowadays, we don't have this syscall,right?
What happens if someone calls it?
I am imagining that TX will fail.
Later, if this syscall exist the tx can be successful. It just need an if strategy to do malicious things.

These are the ForkTriggers @vang1ong7ang tried to explain.

I did not test yet, but we can try that. If that is a call from Native contract the attack will probably be possible..

@Jim8y
Copy link
Contributor Author

Jim8y commented Oct 9, 2023

@vncoelho previous transactions can not call this cause this not yet exist, only if this is added will developers be able to call it, since this will not affect previous transactions at all, no fork will introduce here.

@vncoelho
Copy link
Member

vncoelho commented Oct 9, 2023

If I am right, the attack is very serious, in the same way as reported in #2910, please take some time to understand the previous issue and how critical and dangerous it was.
It was too much imprudent to apply 3.6.0 upgrade to the Consensus Nodes. Let's not be frivolous in conclusions and DO NOT PUT mainnet assets at risk.

It looks like the same issue could happen here if someone perform a malicious transaction before nodes receive the update. Exactly because "can not call this cause this not yet exist". There are combinations/logics that can be done that will cause Fork and even revert Token transactions (if, in the future, a re-sync occurs).

If the attack can proceed, this PR should not be merged without an agreement to definitely solve the issue of updating native functions, etc... (or, at least, use a fork tag precisely connected to the nodes update time). @vang1ong7ang @dusmart.

@Jim8y
Copy link
Contributor Author

Jim8y commented Oct 9, 2023

@vncoelho Oh, I see what you mean now.

@vncoelho
Copy link
Member

vncoelho commented Oct 9, 2023

That is why this needs to be solved before this PR is merged. I did not reject the PR for nothing, there is a reason this should not be merged.

First, solve, then add new features.
Or, create a fork tag.

Otherwise, be prepared because at some point some attack will come...The attack is well known nowadays.

@Jim8y
Copy link
Contributor Author

Jim8y commented Oct 9, 2023

versioning things, consensus version, vm version all matters,,,,,,even plugin versions.........

@Jim8y Jim8y closed this Oct 9, 2023
@Jim8y
Copy link
Contributor Author

Jim8y commented Oct 9, 2023

will reopen after a proper solution to the version is found

@shargon
Copy link
Member

shargon commented Jan 2, 2024

Require #2942

@shargon
Copy link
Member

shargon commented Jan 11, 2024

It will be merged in update-native-contract brach

@shargon shargon merged commit 8c9e0c6 into neo-project:update-native-contracts Jan 11, 2024
1 check passed
@Jim8y Jim8y deleted the adding-Keccak256 branch January 12, 2024 02:16
@Jim8y
Copy link
Contributor Author

Jim8y commented Jan 12, 2024

@AnnaShaleva @roman-khimov neo-go will update accordingly.

@AnnaShaleva
Copy link
Member

Sure, will be implemented together with native contracts update.

AliceInHunterland added a commit to nspcc-dev/neo-go that referenced this pull request Jan 25, 2024
AliceInHunterland added a commit to nspcc-dev/neo-go that referenced this pull request Jan 25, 2024
AliceInHunterland added a commit to nspcc-dev/neo-go that referenced this pull request Jan 25, 2024
AliceInHunterland added a commit to nspcc-dev/neo-go that referenced this pull request Jan 26, 2024
AliceInHunterland added a commit to nspcc-dev/neo-go that referenced this pull request Feb 5, 2024
AliceInHunterland added a commit to nspcc-dev/neo-go that referenced this pull request Feb 8, 2024
AliceInHunterland added a commit to nspcc-dev/neo-go that referenced this pull request Feb 8, 2024
AliceInHunterland added a commit to nspcc-dev/neo-go that referenced this pull request Feb 9, 2024
AliceInHunterland added a commit to nspcc-dev/neo-go that referenced this pull request Feb 9, 2024
shargon added a commit that referenced this pull request Feb 23, 2024
* Update manifest

* Fix comment

* Format & fix methods

* Fix new

* Initialize storage fixes

* Fix IsInitializeBlock

* Fix update

* Fix GetContractState

* Optimize

* Fix first invoke without sync

* Remove current methods

* Clean usings

* Improve reading Initialize

* Clean OnManifestCompose

* Use cache for all native contracts

* Fix ut

* Move cache to ApplicationEngine

* Clean code

* Allow nullable attribute

* Update src/Neo/SmartContract/Native/ContractEventAttribute.cs

* Add base call

* Fix one #2941 (comment)

* Fix IsInitializeBlock #2941 (comment)

* Add ContractEventAttribute constructors for ActiveIn

* Ensure ommited hfs

* Rename

* Case insensitive hf config

* Increase coverage

* More uts

* Rename

* Update src/Neo/SmartContract/Native/ContractManagement.cs

* format code

* Fix ProtocolSettings

* Update src/Neo/SmartContract/ApplicationEngine.cs

* format

* reorder using

* Fix UT

* Adding keccak256 (#2925)

* Create codeql.yml

* Keccak256

* Delete .github/workflows/codeql.yml

* Update src/Neo/SmartContract/Native/CryptoLib.cs

* add more keccak256 test cases as required

* Create codeql.yml

* Keccak256

* Delete .github/workflows/codeql.yml

* Update src/Neo/SmartContract/Native/CryptoLib.cs

* add more keccak256 test cases as required

* HF_Manticore

* Fix copyright

* Create codeql.yml

* Keccak256

* Delete .github/workflows/codeql.yml

* Update src/Neo/SmartContract/Native/CryptoLib.cs

* add more keccak256 test cases as required

* Create codeql.yml

* Keccak256

* Delete .github/workflows/codeql.yml

* Update src/Neo/SmartContract/Native/CryptoLib.cs

* add more keccak256 test cases as required

* HF_Manticore

* Fix copyright

* HF_Manticore

* Update CryptoLib.cs

* Update CryptoLib.cs

* Update UT_CryptoLib.cs

* Apply suggestions from code review

* clean usings

---------

Co-authored-by: Shargon <[email protected]>

* Fix net standard

* Add ut

* Fix update

* Fix update

---------

Co-authored-by: Jimmy <[email protected]>
shargon added a commit that referenced this pull request Mar 2, 2024
* Update manifest

* Fix comment

* Format & fix methods

* Fix new

* Initialize storage fixes

* Fix IsInitializeBlock

* Fix update

* Fix GetContractState

* Optimize

* Fix first invoke without sync

* Remove current methods

* Clean usings

* Improve reading Initialize

* Clean OnManifestCompose

* Use cache for all native contracts

* Fix ut

* Move cache to ApplicationEngine

* Clean code

* Allow nullable attribute

* Update src/Neo/SmartContract/Native/ContractEventAttribute.cs

* Add base call

* Fix one #2941 (comment)

* Fix IsInitializeBlock #2941 (comment)

* Add ContractEventAttribute constructors for ActiveIn

* Ensure ommited hfs

* Rename

* Case insensitive hf config

* Increase coverage

* More uts

* Rename

* Update src/Neo/SmartContract/Native/ContractManagement.cs

* format code

* Fix ProtocolSettings

* Update src/Neo/SmartContract/ApplicationEngine.cs

* format

* reorder using

* Fix UT

* Adding keccak256 (#2925)

* Create codeql.yml

* Keccak256

* Delete .github/workflows/codeql.yml

* Update src/Neo/SmartContract/Native/CryptoLib.cs

* add more keccak256 test cases as required

* Create codeql.yml

* Keccak256

* Delete .github/workflows/codeql.yml

* Update src/Neo/SmartContract/Native/CryptoLib.cs

* add more keccak256 test cases as required

* HF_Manticore

* Fix copyright

* Create codeql.yml

* Keccak256

* Delete .github/workflows/codeql.yml

* Update src/Neo/SmartContract/Native/CryptoLib.cs

* add more keccak256 test cases as required

* Create codeql.yml

* Keccak256

* Delete .github/workflows/codeql.yml

* Update src/Neo/SmartContract/Native/CryptoLib.cs

* add more keccak256 test cases as required

* HF_Manticore

* Fix copyright

* HF_Manticore

* Update CryptoLib.cs

* Update CryptoLib.cs

* Update UT_CryptoLib.cs

* Apply suggestions from code review

* clean usings

---------

Co-authored-by: Shargon <[email protected]>

* Fix net standard

* Add ut

* Fix update

* Fix update

* Expose `GetCommitteeAddress`

* Update src/Neo/SmartContract/Native/NeoToken.cs

---------

Co-authored-by: Jimmy <[email protected]>
AliceInHunterland added a commit to nspcc-dev/neo-go that referenced this pull request Mar 20, 2024
AliceInHunterland added a commit to nspcc-dev/neo-go that referenced this pull request Mar 22, 2024
AnnaShaleva added a commit to nspcc-dev/neo-go that referenced this pull request Apr 8, 2024
With all associated native API changes ported from
neo-project/neo#2925 and
neo-project/neo#3154.

Signed-off-by: Anna Shaleva <[email protected]>
AnnaShaleva added a commit to nspcc-dev/neo-go that referenced this pull request Apr 9, 2024
With all associated native API changes ported from
neo-project/neo#2925 and
neo-project/neo#3154.

Signed-off-by: Anna Shaleva <[email protected]>
AnnaShaleva added a commit to nspcc-dev/neo-go that referenced this pull request Apr 9, 2024
With all associated native API changes ported from
neo-project/neo#2925 and
neo-project/neo#3154.

Signed-off-by: Anna Shaleva <[email protected]>
AnnaShaleva added a commit to nspcc-dev/neo-go that referenced this pull request Apr 10, 2024
With all associated native API changes ported from
neo-project/neo#2925 and
neo-project/neo#3154.

Signed-off-by: Anna Shaleva <[email protected]>
AnnaShaleva added a commit to nspcc-dev/neo-go that referenced this pull request Apr 18, 2024
With all associated native API changes ported from
neo-project/neo#2925 and
neo-project/neo#3154.

Signed-off-by: Anna Shaleva <[email protected]>
AnnaShaleva added a commit to nspcc-dev/neo-go that referenced this pull request Apr 18, 2024
With all associated native API changes ported from
neo-project/neo#2925 and
neo-project/neo#3154.

Signed-off-by: Anna Shaleva <[email protected]>
AnnaShaleva added a commit to nspcc-dev/neo-go that referenced this pull request Apr 18, 2024
With all associated native API changes ported from
neo-project/neo#2925 and
neo-project/neo#3154.

Signed-off-by: Anna Shaleva <[email protected]>
AnnaShaleva added a commit to nspcc-dev/neo-go that referenced this pull request Apr 25, 2024
With all associated native API changes ported from
neo-project/neo#2925 and
neo-project/neo#3154.

Signed-off-by: Anna Shaleva <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked This issue can't be worked at the moment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants