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

Transaction Status RPC #1

Closed
wants to merge 2 commits into from
Closed

Transaction Status RPC #1

wants to merge 2 commits into from

Conversation

MaksymZavershynskyi
Copy link
Contributor

No description provided.

@vgrichina
Copy link

RPC isn't a good match for transaction status. Clients shouldn't poll for status, they should subscribe to status updates and get them pushed to client, using e.g. WebSockets.

@MaksymZavershynskyi
Copy link
Contributor Author

Agree, but one off RPC request should be possible, too.

# Summary
[summary]: #summary

One paragraph explanation of the feature.
Copy link
Member

Choose a reason for hiding this comment

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

improvement, process or standard

@@ -0,0 +1,77 @@
- Feature Name: (fill me in with a unique ident, `my_awesome_feature`)
Copy link
Member

Choose a reason for hiding this comment

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

Proposal Name

@@ -0,0 +1,77 @@
- Feature Name: (fill me in with a unique ident, `my_awesome_feature`)
- Start Date: (fill me in with today's date, YYYY-MM-DD)
- NEP PR: [nearprotocol/neps#0000](https://github.com/nearprotocol/neps/pull/0000)
Copy link
Member

Choose a reason for hiding this comment

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

is this suppose to be updated after you created PR? Probably useful to mention somewhere that process is:

  1. create PR 2) update "NER PR" field

- Feature Name: (fill me in with a unique ident, `my_awesome_feature`)
- Start Date: (fill me in with today's date, YYYY-MM-DD)
- NEP PR: [nearprotocol/neps#0000](https://github.com/nearprotocol/neps/pull/0000)
- Issue: [nearprotocol/<relevant_repo>#0000](https://github.com/nearprotocol/<relevant_repo>/issues/0000)
Copy link
Member

Choose a reason for hiding this comment

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

Should mention that this is not required and also can be many

3. Add your NEP to your fork of the repository. Template TODO.
4. Submit a Pull Request to [this repo](https://github.com/nearprotocol/NEPs).

* For the NEPs repo;
Copy link
Member

Choose a reason for hiding this comment

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

what does this line mean?
was it suppose to be "To contribute to NEPs repo:" without *?

@@ -0,0 +1,118 @@
- Feature Name: `transaction-status`
- Start Date: 2019-06-11
- NEP PR: [nearprotocol/neps#0000](https://github.com/nearprotocol/neps/pull/0000)
Copy link
Member

Choose a reason for hiding this comment

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

incorrect

[motivation]: #motivation

Detailed transaction status is required for good UX as users want the status being updated the second they submit a transaction until this transaction is completed entirely.
All UI tools including console-based tools, like NEAR shell would use this feature.
Copy link
Member

Choose a reason for hiding this comment

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

and Near SDKs (like nearlib)

will return the following (notice how we allow incomplete hash in the RPC):
```json
[
{"ReceivedByNonValidator": {"node_id": "some_node_id"}},
Copy link
Member

Choose a reason for hiding this comment

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

we need a convention here - are we using camel case or underscore.

create overloads of this RPC based on verbosity. So the following RPC call `tx_details` with `{"tx_hash": "04ffa4f"}`
will return the following (notice how we allow incomplete hash in the RPC):
```json
[
Copy link
Member

Choose a reason for hiding this comment

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

This format doesn't really make sense to me.
Who are you going to query to get this information? Are we going to query the whole network to find which nodes see what?
In reality transaction is in:

  • Unknown state
  • In mempool of the given node we query. Here we may have extended mempool for non-validating nodes (right now we don't have that), just so that this nodes can resend transactions if they were not received by current validators (right now it's up to the end client to resend). But the node doesn't really know if it was received (anything that validators says it's not enforceable anyway), so they must decide to resend them based on received chunks back. There is also caveat that this node may not track the shard this TX is aimed at, meaning it actually may not know at all if this Tx was included.
  • included in chunk
  • included in block
    And then for each receipt DFS:
  • receipt included in chunk (e.g. we transition into this, when receipts from previously produced chunks actually got processed by another chard)
  • receipt included in block

[unresolved-questions]: #unresolved-questions

If the user does not own the node that they use to query such information from Wallet or Near shell then it is unclear what
would be the incentive system for such nodes to reply. It is clear that we need some incentive system for non-validating nodes to
Copy link
Member

Choose a reason for hiding this comment

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

the cut of transaction fee for propagation is generally interesting idea, but requires a lot more economics to work. Saito built their whole protocol around such idea actually.

There are problems with censorship and centralization in this case - when big nodes who are validators will be "cheaper" to send to (because they will be validators and won't take the cut of re-trasmission) - so people will send to them and they can freely censor what they want. And there will be high geographic grouping (because server around the corner will be way faster to send TX to), which means political censorship will be enforce on those. Now, you still can send to other nodes in the network, if you think the local one censors you. But it's possible that by the fact that central nodes will become such dominant they will actually gain % of the stake and just push out independent nodes.

@frol
Copy link
Collaborator

frol commented Jun 16, 2019

I would like to put my 2 cents into the RPC & PubSub design. I have designed a few semi-public APIs over the course of last years and my experience is that it is cumbersome to add PubSub into the RPC design after the fact. PubSub requires to manage a ton of connections and keep track of what you should send there.

There are well-established storages for messaging (*MQ [RabbitMQ, ZeroMQ], Kafka), and even feature-complete solutions for PubSub + RPC APIs: WAMP, NATS. I chose to use WAMP for NEAR Explorer, and it works fine there. There is also Rust implementation of WAMP router (i.e. the server which handles the connections), so we can integrate it into nearcore; another approach would be to deploy WAMP router as a standalone component and just push events from nearcore (this keeps nearcore simple to audit, but the deployment gets another moving part).

UPDATE: There is also RSocket from Facebook, but it does not have implementations for Rust and Python (if that matters)

@ilblackdragon ilblackdragon changed the title Add NEP instructions and template taken from Rust RFC. Add first transaction status NEP. Transaction Status RPC Jun 28, 2019
@frol frol mentioned this pull request Oct 18, 2019
8 tasks
@MaksymZavershynskyi
Copy link
Contributor Author

Closing it until we get back to implementing extended transaction status. CC @frol

birchmd added a commit to birchmd/NEPs that referenced this pull request Jun 16, 2023
)

* Rework NEP to be an extension of the account namespaces proposal

* Fix typos

* value_return host function

* Fix typo

* Use WasmTrap instead of generic Panic

* Add list of new host functions to the summary

* Additional notes on NEP-480
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.

4 participants