Skip to content

feat: new rpc `gettxchainlocks' to get transaction statuses by batch #5578

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.

Sign up for GitHub

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

Jump to bottom
Merged
PastaPastaPasta merged 14 commits into from
Sep 20, 2023
Merged

feat: new rpc `gettxchainlocks' to get transaction statuses by batch #5578

PastaPastaPasta merged 14 commits into from
Sep 20, 2023

Conversation

@knst
Copy link
Collaborator

@knst knst commented Sep 17, 2023 •
edited by PastaPastaPasta
Loading

Issue being fixed or feature implemented

Requested by @QuantumExplorer for platform needs

What was done?

New rpc gettransactionsarelocked that returns list of txes.
it does less heavy calculations and transfer less data by gRPC.

How Has This Been Tested?

$ src/dash-cli gettransactionsarelocked  '["e469de7994b9c1da8efd262fee8843efd7bdcab80c700dc1059c98b28f7c5c1b", "0d9fdf00c9568ff9103742b64e6b8287794633072f8824fa2c475f59e71dbace","0d3f48eebead54d640a7fc5692ddfcba619d8b49347d9a7c04586057c02dec9f"]'

[
  {
    "height": 907801,
    "chainlock": true
  },
  {
    "height": 101,
    "chainlock": true
  },
  {
    "height": -1,
    "chainlock": false
  }
]

Limiter tested by this call:

src/dash-cli gettransactionsarelocked  '["", "","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","",""]'  | wc

Breaking Changes

N/A

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated relevant unit/integration/functional/e2e tests
  • I have made corresponding changes to the documentation
  • I have assigned this pull request to a milestone

@knst knst changed the title new rpc getrawtransactions by batch new rpc `gettransactionsarelocked' to get transaction statuses by batch Sep 17, 2023
@knst knst added the RPC Some notable changes to RPC params/behaviour/descriptions label Sep 17, 2023
@knst knst marked this pull request as ready for review September 17, 2023 11:29
@knst knst changed the title new rpc `gettransactionsarelocked' to get transaction statuses by batch feat: new rpc `gettransactionsarelocked' to get transaction statuses by batch Sep 17, 2023
@PastaPastaPasta
Copy link
Member

PastaPastaPasta commented Sep 17, 2023

Based on the commit I pushed format is now

gettransactionsarelocked  '["e469de7994b9c1da8efd262fee8843efd7bdcab80c700dc1059c98b28f7c5c1b", "0d9fdf00c9568ff9103742b64e6b8287794633072f8824fa2c475f59e71dbace","0d3f48eebead54d640a7fc5692ddfcba619d8b49347d9a7c04586057c02dec9f"]'

[
  {
    "height": 907801,
    "chainlock": true
  },
  {
    "height": 101,
    "chainlock": true
  },
  {
    "height": -1,
    "chainlock": false
  }
]

knst
knst commented Sep 17, 2023
View reviewed changes
knst
knst commented Sep 17, 2023
View reviewed changes
Copy link
Collaborator Author

@knst knst left a comment

Choose a reason for hiding this comment

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

utACK, thanks for simplifications @PastaPastaPasta

ogabrielides
ogabrielides previously approved these changes Sep 18, 2023
View reviewed changes
Copy link

@ogabrielides ogabrielides left a comment

Choose a reason for hiding this comment

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

LGTM

thephez
thephez reviewed Sep 18, 2023
View reviewed changes
Copy link
Collaborator

@thephez thephez left a comment

Choose a reason for hiding this comment

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

Will this RPC only return locked transactions? If so I'd propose naming the RPC getlockedtransactions. Or maybe gettransactionslockstatus if it also will return unlocked ones.

UdjinM6
UdjinM6 requested changes Sep 18, 2023
View reviewed changes
Copy link

@UdjinM6 UdjinM6 left a comment

Choose a reason for hiding this comment

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

Now that the amount of returned data is reduced I think we should shrink the diff. Also fixed a couple of other things:

  1. I don't like that we silently cut results
  2. adjusted help text
  3. proposing a different name.

Pls see 1f42ffe

@PastaPastaPasta PastaPastaPasta force-pushed the feature-getrawtransactions branch from a51af6b to 5b70143 Compare September 18, 2023 14:39
thephez
thephez requested changes Sep 18, 2023
View reviewed changes
thephez
thephez approved these changes Sep 19, 2023
View reviewed changes
Copy link
Collaborator

@thephez thephez left a comment

Choose a reason for hiding this comment

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

utACK - help text and RPC name look better to me now 👍

@PastaPastaPasta PastaPastaPasta changed the title feat: new rpc `gettransactionsarelocked' to get transaction statuses by batch feat: new rpc `gettxchainlocks' to get transaction statuses by batch Sep 19, 2023
PastaPastaPasta
PastaPastaPasta previously approved these changes Sep 19, 2023
View reviewed changes
Copy link
Member

@PastaPastaPasta PastaPastaPasta left a comment

Choose a reason for hiding this comment

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

utACK for squash merge

ogabrielides
ogabrielides previously approved these changes Sep 19, 2023
View reviewed changes
Copy link

@ogabrielides ogabrielides left a comment

Choose a reason for hiding this comment

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

utACK

UdjinM6
UdjinM6 requested changes Sep 19, 2023
View reviewed changes
@UdjinM6 UdjinM6 added this to the 20 milestone Sep 19, 2023
@knst knst dismissed stale reviews from ogabrielides and PastaPastaPasta via b1d0467 September 19, 2023 18:28
UdjinM6
UdjinM6 approved these changes Sep 19, 2023
View reviewed changes
Copy link

@UdjinM6 UdjinM6 left a comment

Choose a reason for hiding this comment

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

utACK

PastaPastaPasta
PastaPastaPasta approved these changes Sep 20, 2023
View reviewed changes
Copy link
Member

@PastaPastaPasta PastaPastaPasta left a comment

Choose a reason for hiding this comment

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

utACK for squash merge

@PastaPastaPasta PastaPastaPasta merged commit 633cc32 into dashpay:develop Sep 20, 2023
thephez added a commit to thephez/docs-core that referenced this pull request Oct 3, 2023
@thephez
docs(rpc): add gettxchainlocks
692dd72 
Relates to dashpay/dash#5578
thephez added a commit to dashpay/docs-core that referenced this pull request Oct 3, 2023
@thephez
docs(rpc): more v20 rpc updates (#70)
f827995 
* docs(rpc): update protx diff

Relates to dashpay/dash#5377

* docs(rpc): add getindexinfo rpc

Relates to dashpay/dash#5492

* docs(rpc): add gettxchainlocks

Relates to dashpay/dash#5578

* docs(rpc): update getblock
thephez added a commit to dashpay/docs-core that referenced this pull request Nov 15, 2023
@thephez
docs(rpc): more v20 rpc updates (#70)
89d0399 
* docs(rpc): update protx diff

Relates to dashpay/dash#5377

* docs(rpc): add getindexinfo rpc

Relates to dashpay/dash#5492

* docs(rpc): add gettxchainlocks

Relates to dashpay/dash#5578

* docs(rpc): update getblock
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@UdjinM6 UdjinM6 UdjinM6 approved these changes

@PastaPastaPasta PastaPastaPasta PastaPastaPasta approved these changes

@thephez thephez thephez approved these changes

+1 more reviewer

@ogabrielides ogabrielides ogabrielides left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

RPC Some notable changes to RPC params/behaviour/descriptions

Projects

None yet

Milestone

20

Development

Successfully merging this pull request may close these issues.

5 participants

@knst @PastaPastaPasta @ogabrielides @UdjinM6 @thephez