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

Improve documentation regarding transaction filtering limitations for GetTransactionTrees #1483

Closed
3 tasks
leo-da opened this issue May 31, 2019 · 3 comments
Closed
3 tasks
Assignees
Labels
component/ledger Sandbox and Ledger API

Comments

@leo-da
Copy link
Contributor

leo-da commented May 31, 2019

Rescoped, see #1483 (comment)


GetTransactionTrees GRPC Service does not support TransactionTree filtering by template ID:

Template filtration is not supported on GetTransactionTrees RPC. To get filtered data, use the GetTransactions RPC.

TODOs:

  1. Document GetTransactionTrees GRPC explaining the limitations
  2. Document Java bindings com.daml.ledger.rxjava.TransactionsClient#getTransactionsTrees
  3. Document Scala bindings com.digitalasset.ledger.client.services.transactions.TransactionClient#getTransactionTrees

I also think we should:
a). deprecate the existing GetTransactionTrees GRPC Service:

  // Read the ledger's filtered transaction stream for a set of parties.
  rpc GetTransactions (GetTransactionsRequest) returns (stream GetTransactionsResponse);

  // Read the ledger's complete transaction tree stream for a set of parties.
  rpc GetTransactionTrees (GetTransactionsRequest) returns (stream GetTransactionTreesResponse);

It takes the same GetTransactionsRequest message as GetTransactions service. However it cannot handle template_ids defined in TransactionFilter message.
b). define new messages: GetTransactionTreesRequest and TransactionTreeFilter
c). define a new Transaction Tree service as:

  rpc GetTransactionTreesV2 (GetTransactionTreesRequest) returns (stream GetTransactionTreesResponse);

where GetTransactionTreesRequest message doe NOT allow passing template_ids.

@leo-da leo-da added the component/ledger Sandbox and Ledger API label May 31, 2019
@leo-da
Copy link
Contributor Author

leo-da commented May 31, 2019

Or can we break the backward compatibility?

@bitonic
Copy link
Contributor

bitonic commented Jun 6, 2019

While I agree that the current behavior is not so nice since it's not discoverable from the .proto definitions only, I do not think it's worth breaking backwards compatibility because of this. We've never actually heard a complaint about this, BTW.

However, I just checked and the documentation for this is severely lacking: the .proto files do not even mention this, as far as I can tell. This should definitely be fixed.

@bitonic bitonic changed the title Ledger API TransactionService improvements Improve documentation regarding transaction filtering limitations for GetTransactionTrees Jun 6, 2019
@gerolf-da gerolf-da added this to the Runtime Backlog milestone Feb 14, 2020
@gerolf-da
Copy link
Contributor

The doc for GetTransactionsRequest#filter states that templates must be empty for the GetTransactionTrees method.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/ledger Sandbox and Ledger API
Projects
None yet
Development

No branches or pull requests

3 participants