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

[Core MD] Clarify repo scope: C# implementation ≠ Neo Core protocol #3706

Merged
merged 7 commits into from
Feb 10, 2025

Conversation

Jim8y
Copy link
Contributor

@Jim8y Jim8y commented Feb 1, 2025

Description

This PR formally redefines this repository as an implementation of the Neo Protocol (akin to NeoGo), not the Neo Core itself. It addresses historical conflation between implementation details and protocol authority that has hindered maintenance.

Problem Statement

Since we merged repos into a mono repo, we've conflated the C# implementation with the Neo Core protocol due to:

  1. Undocumented Origins: Early protocol designs were directly coded in C# without formal specs
  2. Authority Fallacy: The absence of reference documentation led to treating this codebase as canonical truth
  3. Scope Bloat: Non-protocol components (CLI, GUI, plugins) became entangled with core protocol logic

This has caused:
⚠️ Maintenance Paralysis: Resistance to modify "core"-labeled non-protocol components
⚠️ Unrealistic Expectations: Assumption that all repo code is protocol-mandated and bug-free
⚠️ Innovation Bottleneck: Difficulty introducing non-protocol improvements

Why This Matters Now
As Neo matures, we must:

  1. Enable protocol evolution without implementation coupling
  2. Decoupling reference specs from any single codebase
  3. Empower contributors to improve tooling without "core" constraints

Type of change

  • Optimization (the change is only an optimization)
  • Style (the change is only a code style for better maintenance or standard purpose)
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

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

Copy link
Contributor

@roman-khimov roman-khimov left a comment

Choose a reason for hiding this comment

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

Do you have a definition of "Neo blockchain protocol"? If not, then what are we implementing here? Until we have this solved this repository is the protocol definition, unfortunately. You can't write an implementation against some standard definition now, you have to follow what is done here.

@@ -6,7 +6,7 @@
</a>
</p>

<h3 align="center">Neo Blockchain</h3>
<h3 align="center">CSharp implementation of the neo blockchain protocol.</h3>
Copy link
Contributor

Choose a reason for hiding this comment

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

And it's Neo, btw. NEO coin, Neo blockchain.

@Jim8y
Copy link
Contributor Author

Jim8y commented Feb 1, 2025

Do you have a definition of "Neo blockchain protocol"? If not, then what are we implementing here? Until we have this solved this repository is the protocol definition, unfortunately. You can't write an implementation against some standard definition now, you have to follow what is done here.

Neo blockchain protocol is another topic either tc can decide or neo foundation can decide. I don't think that can be of any issue for 'csharp implementation'. Similar question can go to any blockchain project, what is ethereum protocol, what is bitcoin protocol, what is sui protocol.

@roman-khimov
Copy link
Contributor

what is ethereum protocol

https://ethereum.github.io/yellowpaper/paper.pdf + EIPs

It's not that I prefer to have an implementation be the definition of the protocol, it's just that we don't have anything else at the moment and we can't ignore this fact.

@Jim8y
Copy link
Contributor Author

Jim8y commented Feb 1, 2025

I thought about it for a while, neo blockchain protocol should be combination of a few subprotocols: neo-gas economy protocol, dbft consensus protocol, neo vm contract/execution protocol. This should be in side of a neo white paper, then extended and detailed in many many NEPs.

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.

Agreed with @roman-khimov

@Jim8y
Copy link
Contributor Author

Jim8y commented Feb 1, 2025

what is ethereum protocol

https://ethereum.github.io/yellowpaper/paper.pdf + EIPs

It's not that I prefer to have an implementation be the definition of the protocol, it's just that we don't have anything else at the moment and we can't ignore this fact.

Neo whitepaper: allcryptowhitepapers.com/neo-whitepaper/

Neo NEPs: https://github.com/neo-project/proposals

protocols are system ruls, it can be verified, analyized, proved.
Implementations are actions to fulfill the rules, it can be run, tested, and benchmarked, but may contain bugs....

This repo implements neo protocol, but it also implements other things that are not part of the protocol.

I understand that the csharp implementation used to be the main place where the design is done, but fact is neo repo also contains many things that should not be considered part of neo protocol, the GUI, the CLI, the plugins. Implementation is implementation, it can have features and tools that is not part of the neo protocol and other implementations do not need to follow.
And when people use that code to run neo, it is not because they trust that implementation has no bugs and works perfectly, but it implements the neo protocol and is provided by default, they can also use neogo if they like, as neogo is another neo protocol implementation.

You guys also wrote many NEPs, so you definately know where the protocols should be.

@vncoelho
Copy link
Member

vncoelho commented Feb 1, 2025

I agree, @Jim8y

But the time has not come yet for that. We need a better protocol specification document that is strictly connected to the NEPS (proposals repository).

After that, I believe that the change in this PR will be more suitable.

@Jim8y
Copy link
Contributor Author

Jim8y commented Feb 1, 2025

I agree, @Jim8y

But the time has not come yet for that. We need a better protocol specification document that is strictly connected to the NEPS (proposals repository).

After that, I believe that the change in this PR will be more suitable.

@vncoelho i am not saying of any big change to any code, just a few lines of specification to this repo..... take a look at those few words change, you will find that nothing actually need to be done for real. Just explicitly tell people that this repo is a csharp implementation of neo protocol.

@cschuchardt88
Copy link
Member

cschuchardt88 commented Feb 1, 2025

@Jim8y is right!

NEPs are for the defining the protocol for neo.

I am sick of moving forward 1 step to just go backwards 10 steps. By developers and community members wanting some new feature built into the protocol. Changes NEEDS to be detailed in a NEP paper in proposal repo. That goes for Notary contract and many many other things. I HATE how a developer doesn't put any thought into the design and implementation in code. Just because @roman-khimov knows how it works and needs to work. Doesn't make it right or ok. We all need to know the protocol for such things (not just developers). All developers say Neo's Protocol is a black box. So where does the standards come from? READING THE CODE. Bad or wrong implementation leads to changes in standardizing neo protocol for non-developers. This is one reason why no one uses neo protocol for bridging funds from other blockchains or anything else related. Also why no contract developers, seeing how you have to read code in order to find out how something works. So please make the necessary changes to stop this behavior. Only than maybe others will use neo protocol, seeing how they can now know what the hell it's protocol is and features are.

@roman-khimov
Copy link
Contributor

That goes for Notary contract

Not a good example for you. There is #1573. And there are numerous links there. You just can't say it was not proposed and explained in a proper manner. The fact that it's not a NEP proper doesn't change much, it's not some randomly coded thing out of nowhere.

Just because @roman-khimov knows how it works and needs to work

Take a look at what @roman-khimov really does in the repo. Things like https://github.com/neo-project/neo/issues?q=is%3Aissue%20state%3Aclosed%20author%3Aroman-khimov. Then check it against https://github.com/neo-project/neo/issues?q=is%3Apr+author%3Aroman-khimov.

Doesn't make it right or ok.

That's for sure. That's why @roman-khimov spends time writing texts from time to time. To communicate problems, intentions, ideas, get feedback, opinions, get on the same page with other people involved basically. Sometimes it works, sometimes it doesn't, communication is hard and reaching consensus among people is much more complex than dBFT.

Just to reiterate, I'm not trying to tell you that we don't need any real specification. I'd be happy to have one, then I wouldn't be touching this repository at all probably. But the fact the we don't have "Neo protocol" defined in any abstract manner makes changes like proposed here just misleading.

@Jim8y
Copy link
Contributor Author

Jim8y commented Feb 2, 2025

roman is doing a great job, lets only focus on changing the repo statement. As a p2p team, we will always find that we dont agree with someone on something, lets try to find the 'grestest common divisor'. i understand roman is saying that currently we need csharp impl to play a role for the core since neps are not complete, that is ok, we just need to not consider the cli gui plugins as the core part. still nothing will be changed, no code need to be updated.

@roman-khimov
Copy link
Contributor

roman-khimov commented Feb 2, 2025

And sometimes we even have a specification, but no implementation for it: https://github.com/neo-project/proposals/blob/f329e8740cb5e19d1d0fba8896d5bd8eb8031b73/nep-25.mediawiki. What's the value of specification in this case? Not a lot of value, no one can use it.

Regarding "non-essential components", yeah, CLI/GUI do not define any of the core protocol, but that's rather understandable. There are less obvious parts like RPC though, in some way it's not core, but just take a look at nspcc-dev/neo-go#2869 or nspcc-dev/neo-go#2863 or nspcc-dev/neo-go#3370 or many many others. Most of them are semantically correct either way, but we have to follow whatever specific choice C# code makes to be 100% compatible.

@cschuchardt88
Copy link
Member

@roman-khimov I wasn't picking on you. Just using you as an example. I know we all play a part. It very hard for me to understand the neo protocol. Your just the really the only one that knows. Because you been here the longest (same for @shargon).

All sounds good to me, I'm down for any improvements.

@Jim8y
Copy link
Contributor Author

Jim8y commented Feb 5, 2025

And sometimes we even have a specification, but no implementation for it: https://github.com/neo-project/proposals/blob/f329e8740cb5e19d1d0fba8896d5bd8eb8031b73/nep-25.mediawiki. What's the value of specification in this case? Not a lot of value, no one can use it.

Regarding "non-essential components", yeah, CLI/GUI do not define any of the core protocol, but that's rather understandable. There are less obvious parts like RPC though, in some way it's not core, but just take a look at nspcc-dev/neo-go#2869 or nspcc-dev/neo-go#2863 or nspcc-dev/neo-go#3370 or many many others. Most of them are semantically correct either way, but we have to follow whatever specific choice C# code makes to be 100% compatible.

@roman-khimov Your concern is totally understandable, yet consider CSharp repo as an implementation will not cause any real trouble at all, since what you consider as the core part will still be the core part, you are still here to review prs, ensuring compatiablility, won't you? so there will not be any problem at all, we still wont change the so called core part without your approval. This pr just make it more clear, stop ambiguous. again NOTHING WILL BE CHANGED AT ALL BECAUSE OF THIS PR.

@roman-khimov
Copy link
Contributor

Well, it's

This pr just make it more clear

vs.

the fact the we don't have "Neo protocol" defined in any abstract manner makes changes like proposed here just misleading

I do not agree it's more clear this way, it's misleading.

@Jim8y
Copy link
Contributor Author

Jim8y commented Feb 6, 2025

Well, it's

This pr just make it more clear

vs.

the fact the we don't have "Neo protocol" defined in any abstract manner makes changes like proposed here just misleading

I do not agree it's more clear this way, it's misleading.

I am sorry, i totally understand your concern, but i still think we need to update it.

misleading if for real, then let it be. problem is problem, and we are here to fix it. If we dont take action, problems will stop each other, A stops B, B stops A.

csharp implementation implements the neo protocol, instead of csharp implementation defines neo protocol.

We must change the mindset. Specification and NEPs are more important than implementations. Otherwise the paradox of this is neo core, you should not change it, if you want to change it, you need to change the neo core first will never end.

@cschuchardt88
Copy link
Member

The current state of this repo is lost. If we are to innovate neo blockchain and be on the leading edge than we need to get away from this repo being neo protocol definition. This has nothing to do about what @Jim8y said above. I would have more luck finding a needle in a haystack than being able to implement the neo protocol with code in this repo. I been here for what 2 years now. Coming in as a fresh pair of eyes. You can trust me to tell, that this needs to be an implementation of the neo protocol. NEP repo is what you should be spending your time on. It will be better for the future of neo. If neo foundation cares at all and I'm sure they do. It would be in the best interest moving forward. I have dealt with projects like this before from other clients. Where the code is so spaghettied out, that you end up spending 99% of the time figuring out about this cascade effect of a bug(s). Because the whole thing is programmed around these bugs that exists in the code base. Fix the bug break everything else. To reiterate use NEP repo for defining neo protocol.

@vncoelho vncoelho dismissed their stale review February 7, 2025 13:44

Specification repo should be re-activated strongly. Not only proposals.

@NGDAdmin NGDAdmin merged commit 00345b1 into neo-project:master Feb 10, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants