jsonrpc: allow parse object at tx status requests #9648

telezhnaya · 2023-10-05T22:04:47Z

Adding the possibility to pass object as params for tx status methods:

{
  "jsonrpc": "2.0",
  "id": "dontcare",
  "method": "EXPERIMENTAL_tx_status", // could be "tx"
  "params": {
    "tx_hash": "6zgh2u9DqHHiXzdy9ouTP7oGky2T4nugqzqt9wJZwNFm",
    "sender_account_id": "sender.testnet"
  }
}

This is just a nice improvement, it looks cleaner.
But I specifically need this as a prep step for #6837
Having json params will allow me to add new request parameter easier.

I thought about adding passing object to send tx methods as well, but the user has to pack it by borsh to be able to sign it.
So, deserialising SignedTransaction by serde makes no sense, I dropped this functionality.
Serialisation can still be useful to be able to dump tx to json so that humans may read it.

wacban

looks good overall and big thanks for adding more tests
I'm a bit worried about renaming fields. Can you clarify if it is safe?

chain/jsonrpc-primitives/src/types/transactions.rs

chain/jsonrpc/src/api/mod.rs

chain/jsonrpc/src/api/transactions.rs

I mixed up adding tests and introducing new functionality here #9648 Please have a look just at tests separately

telezhnaya · 2023-10-09T14:19:27Z

I decided to cover some of things in separate PR #9653

telezhnaya · 2023-10-09T18:02:03Z

@wacban could you please have a look again?
With the tests merged, we are sure I'm not breaking old tests with these changes.

I've also updated the description to be more clear what I'm trying to achieve here.

wacban

LGTM

chain/jsonrpc/src/lib.rs

core/primitives/src/transaction.rs

I mixed up adding tests and introducing new functionality here #9648 Please have a look just at tests separately

telezhnaya · 2023-10-11T15:51:36Z

@wacban could you please have a look on the last commit?

After the discussion with @frol, @firatNEAR and the team, we all came up with the idea that deserialising SignedTransaction by serde makes no sense, because the user has to pack the tx by borsh to be able to sign it.
Please leave the comment that you are still OK with the proposed changes

I mixed up adding tests and introducing new functionality here #9648 Please have a look just at tests separately

Adding the possibility to pass object as params for tx status methods: ``` { "jsonrpc": "2.0", "id": "dontcare", "method": "EXPERIMENTAL_tx_status", // could be "tx" "params": { "tx_hash": "6zgh2u9DqHHiXzdy9ouTP7oGky2T4nugqzqt9wJZwNFm", "sender_account_id": "sender.testnet" } } ``` This is just a nice improvement, it looks cleaner. But I specifically need this as a prep step for #6837 Having json params will allow me to add new request parameter easier. I thought about adding passing object to send tx methods as well, but the user has to pack it by borsh to be able to sign it. So, deserialising SignedTransaction by serde makes no sense, I dropped this functionality. Serialisation can still be useful to be able to dump tx to json so that humans may read it.

I mixed up adding tests and introducing new functionality here #9648 Please have a look just at tests separately

telezhnaya force-pushed the rpc_tx_tests branch from 71089b5 to 17bc1d0 Compare October 5, 2023 22:07

telezhnaya marked this pull request as ready for review October 5, 2023 22:26

telezhnaya requested a review from a team as a code owner October 5, 2023 22:26

telezhnaya requested a review from wacban October 5, 2023 22:26

telezhnaya changed the title ~~rpc: allow parse object at tx status request~~ jsonrpc: allow parse object at tx status request Oct 6, 2023

wacban reviewed Oct 6, 2023

View reviewed changes

telezhnaya mentioned this pull request Oct 9, 2023

jsonrpc: add tx serialisation tests #9653

Merged

github-merge-queue bot pushed a commit that referenced this pull request Oct 9, 2023

jsonrpc: add tx serialisation tests (#9653)

e4e3197

I mixed up adding tests and introducing new functionality here #9648 Please have a look just at tests separately

telezhnaya force-pushed the rpc_tx_tests branch from 82857a9 to 378893e Compare October 9, 2023 17:43

telezhnaya changed the title ~~jsonrpc: allow parse object at tx status request~~ jsonrpc: allow parse object at tx requests Oct 9, 2023

wacban approved these changes Oct 10, 2023

View reviewed changes

chain/jsonrpc/src/lib.rs Show resolved Hide resolved

wacban reviewed Oct 10, 2023

View reviewed changes

core/primitives/src/transaction.rs Outdated Show resolved Hide resolved

nikurt pushed a commit that referenced this pull request Oct 10, 2023

jsonrpc: add tx serialisation tests (#9653)

575c845

I mixed up adding tests and introducing new functionality here #9648 Please have a look just at tests separately

nikurt pushed a commit that referenced this pull request Oct 11, 2023

jsonrpc: add tx serialisation tests (#9653)

2b7de2f

I mixed up adding tests and introducing new functionality here #9648 Please have a look just at tests separately

telezhnaya force-pushed the rpc_tx_tests branch from 378893e to ff0090e Compare October 11, 2023 12:59

telezhnaya changed the title ~~jsonrpc: allow parse object at tx requests~~ jsonrpc: allow parse object at tx status requests Oct 11, 2023

nikurt pushed a commit that referenced this pull request Oct 11, 2023

jsonrpc: add tx serialisation tests (#9653)

f419514

I mixed up adding tests and introducing new functionality here #9648 Please have a look just at tests separately

telezhnaya force-pushed the rpc_tx_tests branch from 58e3141 to f1977c5 Compare October 12, 2023 12:38

jsonrpc: allow parse object at tx requests

ade91e0

telezhnaya force-pushed the rpc_tx_tests branch from f1977c5 to ade91e0 Compare October 12, 2023 12:59

telezhnaya enabled auto-merge October 12, 2023 12:59

telezhnaya added this pull request to the merge queue Oct 12, 2023

Merged via the queue into near:master with commit 22b7a36 Oct 12, 2023
9 checks passed

telezhnaya deleted the rpc_tx_tests branch October 12, 2023 13:47

nikurt pushed a commit that referenced this pull request Oct 16, 2023

jsonrpc: add tx serialisation tests (#9653)

01fef78

I mixed up adding tests and introducing new functionality here #9648 Please have a look just at tests separately

nikurt pushed a commit that referenced this pull request Oct 18, 2023

jsonrpc: add tx serialisation tests (#9653)

a92f055

I mixed up adding tests and introducing new functionality here #9648 Please have a look just at tests separately

nikurt pushed a commit that referenced this pull request Oct 19, 2023

jsonrpc: add tx serialisation tests (#9653)

a44968f

I mixed up adding tests and introducing new functionality here #9648 Please have a look just at tests separately

nikurt pushed a commit that referenced this pull request Oct 19, 2023

jsonrpc: add tx serialisation tests (#9653)

8636dd0

I mixed up adding tests and introducing new functionality here #9648 Please have a look just at tests separately

nikurt pushed a commit that referenced this pull request Oct 19, 2023

jsonrpc: add tx serialisation tests (#9653)

6aec6a8

I mixed up adding tests and introducing new functionality here #9648 Please have a look just at tests separately

nikurt pushed a commit that referenced this pull request Oct 23, 2023

jsonrpc: add tx serialisation tests (#9653)

33b0a7d

I mixed up adding tests and introducing new functionality here #9648 Please have a look just at tests separately

nikurt pushed a commit that referenced this pull request Oct 23, 2023

jsonrpc: add tx serialisation tests (#9653)

e6839de

I mixed up adding tests and introducing new functionality here #9648 Please have a look just at tests separately

nikurt pushed a commit that referenced this pull request Oct 24, 2023

jsonrpc: add tx serialisation tests (#9653)

edffae8

I mixed up adding tests and introducing new functionality here #9648 Please have a look just at tests separately

nikurt pushed a commit that referenced this pull request Oct 24, 2023

jsonrpc: add tx serialisation tests (#9653)

93238bc

I mixed up adding tests and introducing new functionality here #9648 Please have a look just at tests separately

gtsonevv mentioned this pull request Feb 28, 2024

feat: add support for nearcore 1.37 changes LimeChain/near-api-js#12

Closed

5 tasks

gtsonevv mentioned this pull request Mar 12, 2024

Nearcore 1 37 changes near/near-api-js#1317

Merged

5 tasks

petersalomonsen mentioned this pull request Mar 19, 2024

Creating a discussion not working after RPC breaking change NEAR-DevHub/neardevhub-bos#695

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

jsonrpc: allow parse object at tx status requests #9648

jsonrpc: allow parse object at tx status requests #9648

telezhnaya commented Oct 5, 2023 •

edited

Loading

wacban left a comment

telezhnaya commented Oct 9, 2023

telezhnaya commented Oct 9, 2023

wacban left a comment

telezhnaya commented Oct 11, 2023 •

edited

Loading

jsonrpc: allow parse object at tx status requests #9648

jsonrpc: allow parse object at tx status requests #9648

Conversation

telezhnaya commented Oct 5, 2023 • edited Loading

wacban left a comment

Choose a reason for hiding this comment

telezhnaya commented Oct 9, 2023

telezhnaya commented Oct 9, 2023

wacban left a comment

Choose a reason for hiding this comment

telezhnaya commented Oct 11, 2023 • edited Loading

telezhnaya commented Oct 5, 2023 •

edited

Loading

telezhnaya commented Oct 11, 2023 •

edited

Loading