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

Add notary contract #2425

Closed
wants to merge 47 commits into from
Closed

Conversation

doubiliu
Copy link
Contributor

@doubiliu doubiliu commented Apr 6, 2021

Close #1573
This is the C# version, and the original version refers to Neo-go.

@doubiliu
Copy link
Contributor Author

It is here. @devhawk

@devhawk
Copy link
Contributor

devhawk commented Apr 13, 2021

My primary concern about this PR is making sure it fits into the schedule. We are pretty late in the dev process for N3 - I'm not a fan of feature work after the RC portion of the release process. However, @roman-khimov and Stanislav made it pretty clear how important this feature is for NeoFS.

@shargon
Copy link
Member

shargon commented Apr 13, 2021

@neo-project/ngd-shanghai could you give me rights to edit?

@devhawk
Copy link
Contributor

devhawk commented Apr 13, 2021

it feels to me that the notary scenario could be achieved without changes to core

FYI, I'm not sure I still believe this. Notary deals w/ "incomplete" transactions, but there really isn't vocabulary in Neo for incomplete transactions - at least, not that can be communicated to a non-native smart contract. So unless we add mechanisms to the core platform, any notary services would have to be completely off chain. That sounds like a lot of duplicate effort - both duplicating work that the core platform is already doing as well as duplicating code/infrastrucutre across separate dApp projects.

There isn't really a mechanism in the platform to represent an inco

@erikzhang
Copy link
Member

What about we build a webapi, and setup the Uri on chain by the committee? I think this method is also decentralized and simpler and more efficient. More importantly, it does not need to modify the core protocol.

@roman-khimov
Copy link
Contributor

  1. Centralized webapi is inevitably a single point of failure. We're building distributed decentralized applications, so having it in the loop seem to contradict the essence of these applications. This service could always go down for its own reasons, but at the same time it'd be trivial to launch a DDOS attack against it affecting all dApps using this service. Protecting against these attacks requires substantial efforts and costs some real money, which leads us to the next problem.
  2. This service has to be run by someone, it's not free both in terms of human resources and hardware cost. So the question is --- why would they do that? There has to be some economic incentive for operators of this service.
  3. It doesn't cover all of use-cases that can be covered by Notary system. An example is contract-sponsored transactions, centralized service hides all payloads from the network, so the network can't react on them.

Compare that to the Notary scheme:

  1. It can work with just one notary node left on the network, so if we're to assign some 7 nodes to do the job we'll have nice redundancy and all of these nodes will operate on P2P network, so picking and targeting any of them individually is not trivial.
  2. There is an economic model behind it, node operators receive some GAS for performing signature collection, so it covers the cost of running the node and motivates operators to maintain their nodes. This incentive also brings more nodes to the network in general which is also important, it's another role that can be serviced by another set of nodes. Having a healthy number of independent nodes is crucial for the network and Notary system can contribute to that.
  3. As all payloads are visible on P2P network it's not hard to add some additional logic reacting on them like completing some contract-sponsored transactions. We actually want to do that for NeoFS, because we need nodes to have some GAS to operate on the NeoFS network and distributing GAS for each of them is not trivial, so what we'd like to do in future is exactly that, make nodes send incomplete transactions with special sender that owns a lot of GAS, monitor their appearance on the network in the service and then complete these transactions by adding a signature.

The most important thing is that this subsystem with all of its features can be used by any dApp, the same set of problems we're solving for NeoFS is also relevant for any applications that have multiple parties interacting (like multiplayer games, for example). Modifying the core protocol is not easy, but there are cases that can't be reliably solved without doing that and I think we have a compelling reason here, we're enabling the network to do something that it couldn't do before.

@shargon
Copy link
Member

shargon commented Apr 14, 2021

There is an economic model behind it, node operators receive some GAS for performing signature collection, so it covers the cost of running the node and motivates operators to maintain their nodes. This incentive also brings more nodes to the network in general which is also important, it's another role that can be serviced by another set of nodes. Having a healthy number of independent nodes is crucial for the network and Notary system can contribute to that.

For me, this is the key point, as I said it could be done without extra nodes, but maybe this is worst, as you said, it could bring more nodes to the network, but this is an economic area where I am not expert

@satoshichou
Copy link

satoshichou commented Apr 16, 2021

I like the idea of notary node although it's not perfect, I think we need varied ecnomic models for different role mantainers. That give more senarios for gas and encourage people to come in to neo. We can do everything from centralized service, but it's no use for commity growth. I like neo, if neo could benifit people, people will benefit Neo Network much more. I also had a look of NeoFS, notary node is very important as a module for it's system. If you guys want to make it centralize, how could it be open source and when it will be set. I think you guys want it to be intergrated to RC2 right?

@ZhangTao1596
Copy link
Contributor

ZhangTao1596 commented Apr 20, 2021

@erikzhang said he will approve this solution only if we don't add new p2p message type and transaction attribute.
As for P2P, I think we can use ExtensiblePayload. Accounts that has deposit enough gas in NotaryContract can be allowed to send this payload and if they spam the network, notarys can punish them.
How do you think?@roman-khimov

@roman-khimov
Copy link
Contributor

As for P2P, I think we can use ExtensiblePayload. Accounts that has deposit enough gas in NotaryContract can be allowed to send this payload and if they spam the network, notarys can punish them.

Currently Extensible payloads are tightly coupled with RelayCache and if we're to allow anyone to send them with Notary subtype inside it'll blow up like in #2432. So we absolutely need some separate pool that works the way current Notary payload pool does and if we're to have that then I'm not sure how other current Extensible payloads would fit into this pool. Also, current Extensible payloads are a kind of opaque data sent between a limited set of addresses, so this wrapper has some common logic for this and Notary payloads don't really fit in there, because regular nodes do need to look inside and ensure correctness of these payloads (just like they ensure transaction correctness before relaying).

So it probably can be done, but we'll need to ensure that:

  • Notary payloads are limited by deposit
  • they don't interfere with consensus messages
  • all nodes ensure Notary payload correctness

Which to me equals to rewriting Extensible payload handling completely and I'm not sure we want that.

As for attributes, I don't think we can do anything meaningful without them.

Conflicts is actually a feature of its own (#1991) and I think it's useful even without any notaries, although in Notary system it ensures that either main transaction gets in or fallback one, but not both. Remember that Notary subsystem allows to collect all signatures in sub-block time (and that's how it works right now on RC1 NeoFS testnet), so this conflict has to be resolved before block acceptance and as was discussed in #1991 there doesn't seem to be any way to do that without this attribute.

NotValidBefore (#1992) is a very cheap (implementation-wise) attribute, it just complements ValidUntilBlock logic. If we're to remove it notaries can send fallback transactions without even trying to complete the main one. It's not in their best interest (because completing main transaction gives them a bit more GAS in fees), but at the same time it's less work and it allows to get GAS faster (who knows when this main transaction will be completed?). And given that fallback transaction can be completed by any of notary nodes any rogue one can just ruin the system for fun if we're to remove this attribute. So it's important for creating this guaranteed window of time where no notary node (good or bad one) can use fallback transaction, and while they can't use fallback they'll probably try to complete the main one to get more GAS and to get it faster.

NotaryAssisted attribute is important for proper fee collection/distribution, while we can check for notary contract signer presence to see if transaction has used notary subsystem it's important to also know the number of signatures collected. If we're to treat any notary-assisted transaction equal then it'd be economically more appealing for notary nodes to never complete transactions and instead of sending one main transaction (receiving one FEE) just send seven fallbacks (and get 7×FEE).

@roman-khimov
Copy link
Contributor

Meanwhile, @alexvanin is testing mainnet NeoFS contract with Notary subsystem enabled: nspcc-dev/neofs-contract#74. It simplifies the contract obviously, but there is even more interesting observation that directly affects future mainnet operations (where GAS costs something).

NeoFS contract accepts GAS from data owners that over time gets transferred to storage node owners that then probably would like to withdraw this GAS from the contract to their accounts. Withdrawing requires BFT number of signatures from alphabet nodes and on-chain signature collection used by current NeoFS contract leads to a number of transactions (7 for 7 alphabet nodes) with a total cost of 2.8-7.0 GAS. So you just can't withdraw any GAS from NeoFS contract without paying this fee and this fee is quite huge. Notary-enabled contract allows to withdraw with just 0.2 GAS which seems to be reasonable.

So it's not just the number of transactions problem that we're solving here, it's also critically important for mainnet NeoFS economics viability.

@ZhangTao1596
Copy link
Contributor

ZhangTao1596 commented Apr 21, 2021

Can we assume notary nodes are completely trustworthy, Since they are designated by committee? So we won't need Conflict and NotValidBefore.

@roman-khimov
Copy link
Contributor

Can we assume notary nodes are completely trustworthy

I don't think we can assume this even for CNs (otherwise we don't need BFT consensus, there are simpler algorithms for trusted nodes). And notaries can misbehave in different ways, not just for malicious intent, but because of some bug for example. Removing these attributes is dangerous, one misbehaving node could ruin whole service then and I don't think we can accept this risk.

@devhawk devhawk mentioned this pull request Jul 2, 2021
24 tasks
/// <summary>
/// Notary nodes
/// </summary>
Notary = 128
Copy link
Member

Choose a reason for hiding this comment

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

I believe it is complex as discussed but may be worth because each time more we need services for multi-signatures aggregation.

@roman-khimov
Copy link
Contributor

We'd like to discuss some minor adjustments to the notary subsystem that can/should be done as a part of this PR. They're not critical, they don't change the logic, but still they can be considered for better future extensibility of the protocol.

  1. Magic numbers used
    There is a number of IDs introduced by the notary subsystem, at the moment it tries to stay away from "official" ones as much as it can (like in Reserve some TransactionAttributeTypes for experimental/private usage #1904), but they were meant to be replaced by more proper values. A list of IDs:

    • role number used by RoleManagement contract, currently 128, but the next free one in sequence of roles is 32
    • NotValidBefore, Conflicts and NotaryAssisted transaction attributes, currently using 0xe0, 0xe1 and 0xe2, not sure which ones are better for them, we only have two other attribute types; maybe 0x20, 0x21 and 0x22
    • P2P message type for notary requests, 0x50 now, maybe 0x26 will keep it in 0x2x range, though we can keep 0x50 as well and reserve some other range for experiments
  2. FEE value
    Notary-assisted attribute fee value is hardcoded to 0.1 GAS, probably it should be configurable via Notary contract (like getPrice/setPrice in the Oracle contract). 0.1 still seems to be a good one for initial/default value (given the default ExecFeeFactor), but for public networks with their current ExecFeeFactor we can lower it to 0.02 or 0.01.

Of course we'll also need to enable the new contract at the appropriate height for each network, but it should already be possible with NativeUpdateHistory.

@ZhangTao1596
Copy link
Contributor

Close this because of #2661 .

@doubiliu doubiliu closed this Jun 16, 2022
@shargon shargon deleted the addNotaryContract branch April 8, 2024 07:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Network assistance for multisignature transaction forming
10 participants