This repository has been archived by the owner on Apr 17, 2019. It is now read-only.
Add GetPendingTransactions query to shared model #1532
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
l4l
previously requested changes
Jul 3, 2018
docs/source/api/queries.rst
Outdated
| :header: "Field", "Description", "Constraint", "Example" | ||
| :widths: 15, 30, 20, 15 | ||
|
|
||
| "Transactions", "an array of transactions for given account", "Committed transactions", "{tx1, tx2…}" |
There was a problem hiding this comment.
Or "list of unsigned transactions or batches"
There was a problem hiding this comment.
Unsigned is not really a good word here. Becuase [valid] multisig txes either signed or haven't been sent yet
| #ifndef IROHA_QUERY_EXECUTION_HPP | ||
| #define IROHA_QUERY_EXECUTION_HPP | ||
|
|
||
| #include "ametsuchi/block_query.hpp" | ||
| #include "ametsuchi/wsv_query.hpp" | ||
| #include "builders/protobuf/builder_templates/query_response_template.hpp" | ||
| #include "builders/protobuf/builder_templates/blocks_query_template.hpp" |
There was a problem hiding this comment.
You are right. This was redundant.
| QueryProcessingFactory::executeGetPendingTransactions( | ||
| const shared_model::interface::GetPendingTransactions &query, | ||
| const shared_model::interface::types::AccountIdType &query_creator) { | ||
| std::vector<shared_model::proto::Transaction> txs; |
There was a problem hiding this comment.
I think it is temporary unless query execution is implemented. Please leave a comment about that
There was a problem hiding this comment.
Empty for now.
Currently, the query does NOT perform actual information retrieving (will be implemented after appearing of transactions batches support in develop branch).
There was a problem hiding this comment.
Sorry, haven't noticed that. Maybe you consider implementing mst? It's already implemented
There was a problem hiding this comment.
It is ok. Now we have common understanding. TODO about missing implementation is placed on a line below.
kamilsa
previously requested changes
Jul 4, 2018
docs/source/api/queries.rst
Outdated
| :header: "Field", "Description", "Constraint", "Example" | ||
| :widths: 15, 30, 20, 15 | ||
|
|
||
| "Transactions", "an array of transactions for given account", "Committed transactions", "{tx1, tx2…}" |
There was a problem hiding this comment.
Or "list of unsigned transactions or batches"
| QueryProcessingFactory::executeGetPendingTransactions( | ||
| const shared_model::interface::GetPendingTransactions &query, | ||
| const shared_model::interface::types::AccountIdType &query_creator) { | ||
| std::vector<shared_model::proto::Transaction> txs; |
There was a problem hiding this comment.
I think it is temporary unless query execution is implemented. Please leave a comment about that
igor-egorov
dismissed stale reviews from l4l and kamilsa
Requested changes were applied.
l4l
approved these changes
Jul 4, 2018
| @@ -192,6 +182,16 @@ of signatures required to consider a transaction signed. | |||
| The default value is 1. | |||
| Each account can link additional public keys and increase own quorum number. | |||
|
|
|||
| Role | |||
There was a problem hiding this comment.
Could you please revert reordering, it differs from the pr aim
There was a problem hiding this comment.
Hmm. It was an intended change. Now the topics are sorted alphabetically if no other more valuable factors involved (e.g. logical composition).
kamilsa
suggested changes
Jul 4, 2018
| Atomic Batch | ||
| ------------ | ||
|
|
||
| All the transactions within an atomic batch should pass `stateful validation`_ for the batch to be applied to a ledger. |
There was a problem hiding this comment.
Please add sentence saying that order of transactions in atomic batch is preserved
kamilsa
approved these changes
Jul 4, 2018
igor-egorov
force-pushed
the
feature/query-pending-transactions
branch
2 times, most recently
from
be27b5a
to
1082bee
Compare
added 3 commits
July 6, 2018 20:08
Now the query returns empty list of transactions. Later the behavior will be also implemented. Signed-off-by: Igor Egorov <igor@soramitsu.co.jp>
Signed-off-by: Igor Egorov <igor@soramitsu.co.jp>
Signed-off-by: Igor Egorov <igor@soramitsu.co.jp>
igor-egorov
force-pushed
the
feature/query-pending-transactions
branch
from
1082bee
to
5b2edbb
Compare
stinger112
pushed a commit
to stinger112/iroha
that referenced
this pull request
Jul 19, 2018
…s#1532) This PR adds an interface of GetPendingTransactions query to shared model. Currently, the query does NOT perform actual information retrieving (will be implemented after appearing of transactions batches support in develop branch). Additionally, this pr provides documentation for the new query. It is a starting point for further development of GetPendingTransactions feature. Signed-off-by: Igor Egorov <igor@soramitsu.co.jp>
l4l
pushed a commit
that referenced
this pull request
Jul 25, 2018
This PR adds an interface of GetPendingTransactions query to shared model. Currently, the query does NOT perform actual information retrieving (will be implemented after appearing of transactions batches support in develop branch). Additionally, this pr provides documentation for the new query. It is a starting point for further development of GetPendingTransactions feature. Signed-off-by: Igor Egorov <igor@soramitsu.co.jp>
l4l
pushed a commit
that referenced
this pull request
Jul 25, 2018
This PR adds an interface of GetPendingTransactions query to shared model. Currently, the query does NOT perform actual information retrieving (will be implemented after appearing of transactions batches support in develop branch). Additionally, this pr provides documentation for the new query. It is a starting point for further development of GetPendingTransactions feature. Signed-off-by: Igor Egorov <igor@soramitsu.co.jp>
l4l
pushed a commit
that referenced
this pull request
Jul 25, 2018
This PR adds an interface of GetPendingTransactions query to shared model. Currently, the query does NOT perform actual information retrieving (will be implemented after appearing of transactions batches support in develop branch). Additionally, this pr provides documentation for the new query. It is a starting point for further development of GetPendingTransactions feature. Signed-off-by: Igor Egorov <igor@soramitsu.co.jp>
l4l
pushed a commit
that referenced
this pull request
Jul 25, 2018
This PR adds an interface of GetPendingTransactions query to shared model. Currently, the query does NOT perform actual information retrieving (will be implemented after appearing of transactions batches support in develop branch). Additionally, this pr provides documentation for the new query. It is a starting point for further development of GetPendingTransactions feature. Signed-off-by: Igor Egorov <igor@soramitsu.co.jp>
l4l
pushed a commit
that referenced
this pull request
Jul 25, 2018
This PR adds an interface of GetPendingTransactions query to shared model. Currently, the query does NOT perform actual information retrieving (will be implemented after appearing of transactions batches support in develop branch). Additionally, this pr provides documentation for the new query. It is a starting point for further development of GetPendingTransactions feature. Signed-off-by: Igor Egorov <igor@soramitsu.co.jp>
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description of the Change
This PR adds an interface of GetPendingTransactions query to shared model.
Currently, the query does NOT perform actual information retrieving (will be implemented after appearing of transactions batches support in develop branch).
Additionally, this pr provides documentation for the new query.
Benefits
It is a starting point for further development of GetPendingTransactions feature.
Possible Drawbacks
None ?
Usage Examples or Tests
build everything and test everything, even bindings