Implement `getrawnotaryrequest` and `getrawnotarytransaction` RPC extensions #3098

tatiana-nspcc · 2023-08-22T11:47:02Z

Problem

We have notary pool subscriptions, but it's not enough for debugging some unexpected things that may happen while submitting notary requests.
The problem is described in #2951

Solution

Add the following RPC extensions:

getRawNotaryPool returns hashes from all the verified transactions, including both main and fallback transactions.
getRawNotaryRequest takes a transaction hash and attempts to find the corresponding transaction in the notary requests mempool. It searches through all the verified main and fallback transactions.

Close #2951.

codecov · 2023-08-22T11:56:00Z

Codecov Report

Merging #3098 (7afa950) into master (f330aaf) will decrease coverage by 0.06%.
Report is 4 commits behind head on master.
The diff coverage is 90.52%.

@@            Coverage Diff             @@
##           master    #3098      +/-   ##
==========================================
- Coverage   84.89%   84.83%   -0.06%     
==========================================
  Files         329      330       +1     
  Lines       44239    44334      +95     
==========================================
+ Hits        37555    37611      +56     
- Misses       5172     5211      +39     
  Partials     1512     1512

Files Changed	Coverage Δ
pkg/neorpc/result/raw_notary_pool.go	`70.00% <70.00%> (ø)`
pkg/rpcclient/rpc.go	`82.71% <88.46%> (+0.17%)`	⬆️
pkg/core/mempool/mem_pool.go	`97.25% <100.00%> (+0.05%)`	⬆️
pkg/services/rpcsrv/server.go	`80.92% <100.00%> (+0.33%)`	⬆️

... and 4 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

pkg/services/rpcsrv/server.go

pkg/services/rpcsrv/server_test.go

AnnaShaleva

The comment about PR structure:

Change the PR name, it should describe what you are implementing/optimizing/fixing/doing in this PR. What you are doing here is supporting the getrawnotarypool and getrawnotarytransaction RPC extensions. Thus, PR may be named "implement getrawnotaryrequest and getrawnotarytransaction RPC extensions".
Adjust the PR description: it should contain any of "Close Add getrawnotaryrequestpool RPC extension #2951." or "Closes Add getrawnotaryrequestpool RPC extension #2951." or "Fixes Add getrawnotaryrequestpool RPC extension #2951." keywords to let the GitHub know that your PR is not just references the initial issue, but closes it. I.e. "The problem is described in Add getrawnotaryrequestpool RPC extension #2951" is not enough for this.

Note, that these comments are relevant for any other PR.

Note about PR content:

We should support the same methods for RPC Client.
Documentation should be updated (a part about RPC extensions).
New mempool APIs should be tested (but this can be doe after APIs finalisation).

pkg/neorpc/result/raw_notary_pool.go

pkg/services/rpcsrv/server.go

pkg/core/mempool/mem_pool.go

pkg/services/rpcsrv/server_test.go

pkg/services/rpcsrv/server.go

pkg/rpcclient/rpc.go

pkg/core/mempool/mem_pool.go

pkg/core/mempool/mem_pool_test.go

pkg/rpcclient/doc.go

docs/rpc.md

pkg/core/mempool/mem_pool_test.go

pkg/neorpc/result/raw_notary_pool.go

pkg/services/rpcsrv/server.go

pkg/rpcclient/rpc.go

pkg/rpcclient/doc.go

docs/rpc.md

Forbid Notary service to change the fallback's witnesses in any way. Fix the problem described in #3098 (comment). Signed-off-by: Anna Shaleva <[email protected]>

Forbid Notary service to change the fallback's witnesses in any way. Fix the problem described in review comment: #3098 (comment). Signed-off-by: Anna Shaleva <[email protected]>

pkg/neorpc/result/raw_notary_pool.go

pkg/services/rpcsrv/server.go

pkg/services/rpcsrv/server_test.go

pkg/rpcclient/rpc_test.go

pkg/neorpc/result/raw_notary_pool.go

pkg/services/rpcsrv/server_test.go

pkg/rpcclient/rpc_test.go

pkg/services/rpcsrv/client_test.go

AnnaShaleva · 2023-08-31T08:06:10Z

Otherwise LGTM.

AnnaShaleva

LGTM.

@roman-khimov, could you review?
@tatiana-nspcc, please, convert the PR from draft.

AnnaShaleva · 2023-08-31T14:48:10Z

@tatiana-nspcc, could you also rebase onto the current master, please?

tatiana-nspcc · 2023-08-31T14:51:05Z

It was rebased before the last push. As I see it's still based on a current master state.

AnnaShaleva · 2023-08-31T14:53:43Z

It was rebased before the last push. As I see it's still based on a current master state.

That's true, which means I broke our CodeQL job in the #3113.

roman-khimov

Perfectly fine otherwise.

pkg/core/mempool/mem_pool_test.go

docs/rpc.md

IterateVerifiedTransactions iterates through verified transactions in memory pool and invokes function cont. Where cont callback returns whether we should continue with the traversal process. Signed-off-by: Tatiana Nesterenko <[email protected]>

`getrawnotarytransaction` takes a transaction hash and attempts to find the corresponding transaction in the notary requests mempool. It searches through all the verified main and fallback transactions. `getrawnotarypool` returns hashes of all the verified transactions, including both main and fallback transactions. Additionally add struct result.RawNotaryPool. Close #2951 Signed-off-by: Tatiana Nesterenko <[email protected]>

…ethods GetRawNotaryTransaction returns a fallback or main transaction that was previously added to the memory pool by P2PNotaryRequest. This function invokes the RPC server's `getrawnotarytransaction` method. GetRawNotaryPool returns hashes from all the verified transactions, including both main and fallback transactions. This function invokes the RPC server's `getrawnotarypool` method. Also, these functions were added to doc.go. Signed-off-by: Tatiana Nesterenko <[email protected]>

Signed-off-by: Tatiana Nesterenko <[email protected]>

tatiana-nspcc requested review from AnnaShaleva and roman-khimov as code owners August 22, 2023 11:47

tatiana-nspcc marked this pull request as draft August 22, 2023 11:51

tatiana-nspcc force-pushed the 2951-getrawnotarypool branch from 3918670 to 0ec25db Compare August 22, 2023 11:55

tatiana-nspcc commented Aug 22, 2023

View reviewed changes

pkg/services/rpcsrv/server.go Outdated Show resolved Hide resolved

tatiana-nspcc commented Aug 23, 2023

View reviewed changes

pkg/services/rpcsrv/server_test.go Outdated Show resolved Hide resolved

AnnaShaleva reviewed Aug 23, 2023

View reviewed changes

tatiana-nspcc force-pushed the 2951-getrawnotarypool branch 2 times, most recently from 565296b to 3bb20bb Compare August 27, 2023 19:16

tatiana-nspcc commented Aug 27, 2023

View reviewed changes

pkg/services/rpcsrv/server.go Show resolved Hide resolved

tatiana-nspcc commented Aug 27, 2023

View reviewed changes

pkg/rpcclient/rpc.go Show resolved Hide resolved

tatiana-nspcc changed the title ~~getRawNotaryPool and getRawNotaryRequest~~ Implement getrawnotaryrequest and getrawnotarytransaction RPC extensions Aug 27, 2023

tatiana-nspcc requested a review from AnnaShaleva August 28, 2023 06:10

AnnaShaleva reviewed Aug 28, 2023

View reviewed changes

tatiana-nspcc force-pushed the 2951-getrawnotarypool branch 2 times, most recently from c7ead95 to 9f8f44c Compare August 28, 2023 17:04

tatiana-nspcc requested a review from AnnaShaleva August 28, 2023 17:07

AnnaShaleva reviewed Aug 29, 2023

View reviewed changes

AnnaShaleva mentioned this pull request Aug 29, 2023

notary: avoid changing pooled fallback transaction witnesses #3108

Merged

tatiana-nspcc force-pushed the 2951-getrawnotarypool branch from 9f8f44c to 7f1d872 Compare August 29, 2023 22:59

tatiana-nspcc requested a review from AnnaShaleva August 29, 2023 23:07

AnnaShaleva reviewed Aug 30, 2023

View reviewed changes

tatiana-nspcc force-pushed the 2951-getrawnotarypool branch 3 times, most recently from 678d021 to 5a1b5bc Compare August 31, 2023 06:48

tatiana-nspcc requested a review from AnnaShaleva August 31, 2023 06:51

AnnaShaleva reviewed Aug 31, 2023

View reviewed changes

tatiana-nspcc force-pushed the 2951-getrawnotarypool branch from 5a1b5bc to cff45b4 Compare August 31, 2023 14:36

AnnaShaleva approved these changes Aug 31, 2023

View reviewed changes

tatiana-nspcc marked this pull request as ready for review August 31, 2023 14:48

roman-khimov reviewed Aug 31, 2023

View reviewed changes

pkg/core/mempool/mem_pool_test.go Outdated Show resolved Hide resolved

docs/rpc.md Outdated Show resolved Hide resolved

tatiana-nspcc added 4 commits August 31, 2023 18:51

docs: add getrawnotarypool, getrawnotarytransaction

7afa950

Signed-off-by: Tatiana Nesterenko <[email protected]>

tatiana-nspcc force-pushed the 2951-getrawnotarypool branch from cff45b4 to 7afa950 Compare August 31, 2023 18:40

roman-khimov merged commit 0d30c83 into master Aug 31, 2023
13 of 18 checks passed

roman-khimov deleted the 2951-getrawnotarypool branch August 31, 2023 19:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement `getrawnotaryrequest` and `getrawnotarytransaction` RPC extensions #3098

Implement `getrawnotaryrequest` and `getrawnotarytransaction` RPC extensions #3098

tatiana-nspcc commented Aug 22, 2023 •

edited

Loading

codecov bot commented Aug 22, 2023 •

edited

Loading

AnnaShaleva left a comment

AnnaShaleva commented Aug 31, 2023

AnnaShaleva left a comment

AnnaShaleva commented Aug 31, 2023

tatiana-nspcc commented Aug 31, 2023

AnnaShaleva commented Aug 31, 2023

roman-khimov left a comment

Implement getrawnotaryrequest and getrawnotarytransaction RPC extensions #3098

Implement getrawnotaryrequest and getrawnotarytransaction RPC extensions #3098

Conversation

tatiana-nspcc commented Aug 22, 2023 • edited Loading

Problem

Solution

codecov bot commented Aug 22, 2023 • edited Loading

Codecov Report

AnnaShaleva left a comment

Choose a reason for hiding this comment

AnnaShaleva commented Aug 31, 2023

AnnaShaleva left a comment

Choose a reason for hiding this comment

AnnaShaleva commented Aug 31, 2023

tatiana-nspcc commented Aug 31, 2023

AnnaShaleva commented Aug 31, 2023

roman-khimov left a comment

Choose a reason for hiding this comment

Implement `getrawnotaryrequest` and `getrawnotarytransaction` RPC extensions #3098

Implement `getrawnotaryrequest` and `getrawnotarytransaction` RPC extensions #3098

tatiana-nspcc commented Aug 22, 2023 •

edited

Loading

codecov bot commented Aug 22, 2023 •

edited

Loading