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.

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

RPC end point to configure broadcast transaction await #6837

Closed
ilblackdragon opened this issue May 19, 2022 · 2 comments · Fixed by #9644
Closed

RPC end point to configure broadcast transaction await #6837

ilblackdragon opened this issue May 19, 2022 · 2 comments · Fixed by #9644
Assignees
@telezhnaya
Labels
A-RPC Area: rpc C-enhancement Category: An issue proposing an enhancement or a PR with one. T-integrations Team: issues relevant to the integrations team

Comments

@ilblackdragon
Copy link
Member

ilblackdragon commented May 19, 2022 •
edited by exalate-issue-sync bot
Loading

Describe the use case Currently there is only option to either broadcast tx or broadcast and wait for finality. There should be a way to configure what user wants to wait when broadcasting tx.

Proposed solution Deprecating both broadcast_tx_async and boardcast_tx_commit and replacing it with a new configurable broadcast:

enum BroadcastWait {
 /// Returns right away.
 None,
 /// Waits only for inclusion into the block.
 Inclusion,
 /// Waits until block with inclusion is finalized.
 InclusionFinal,
 /// Waits until all non-refund receipts are executed.
 Executed,
 /// Waits until all non-refund receipts are executed and the last of the blocks is final.
 ExecutedFinal,
 /// Waits until everything has executed and is final.
 Final,
 }
@ilblackdragon ilblackdragon changed the title RPC end point to broadcast tx and wait for inclusion RPC end point to configure broadcast transaction await May 19, 2022
@bowenwang1996 bowenwang1996 added A-RPC Area: rpc C-enhancement Category: An issue proposing an enhancement or a PR with one. labels May 19, 2022
@firatNEAR firatNEAR added the T-integrations Team: issues relevant to the integrations team label May 24, 2022
@firatNEAR firatNEAR removed the N2 label Oct 25, 2022
@austinabell austinabell removed their assignment Jan 8, 2023
@telezhnaya telezhnaya assigned telezhnaya and unassigned firatNEAR Sep 14, 2023
@telezhnaya
Copy link
Contributor

In my opinion, we want to redesign tx method as well
https://docs.near.org/api/rpc/transactions#transaction-status

Now the doc says "please go and check the statuses of all receipts, the finality of the blocks, etc"
It would be useful to take this responsibility on us

This was referenced Sep 19, 2023
Merged
Closed
Merged
github-merge-queue bot pushed a commit that referenced this issue Oct 4, 2023
@telezhnaya
add status field to response in RPC methods tx, `EXPERIMENTAL_tx_st…
9c8d0bb 
…atus`, `broadcast_tx_commit`, `broadcast_tx_async` (#9556)

This PR is a part of RPC tx methods redesign
https://docs.google.com/document/d/1jGuXzBlMAGQENsUpy5x5Xd7huCLOd8k6tyMOEE41Rro/edit?usp=sharing
Addressing point 3 in #9542 and
start working on #6837

Changes:
- New field `status` appeared in the response of RPC methods `tx`,
`EXPERIMENTAL_tx_status`, `broadcast_tx_commit`, `broadcast_tx_async`
- Added some comments with the explanations

In #6837, Illia suggested to have
the structure
```rust
enum BroadcastWait {
 /// Returns right away.
 None,
 /// Waits only for inclusion into the block.
 Inclusion,
 /// Waits until block with inclusion is finalized.
 InclusionFinal,
 /// Waits until all non-refund receipts are executed.
 Executed,
 /// Waits until all non-refund receipts are executed and the last of the blocks is final.
 ExecutedFinal,
 /// Waits until everything has executed and is final.
 Final,
 }
```

I've renamed it to `TxExecutionStatus` and simplified it a little by
dropping all refund options:
1. We may add them later without breaking backwards compatibility
2. To support them, we need to have the access to `receipt` (it's the
method `get_tx_execution_status`). We have there only
`execution_outcome` by default. We created special logic to be able not
to retrieve the receipts (it's the only difference between `tx` and
`EXPERIMENTAL_tx_status`). I have a feeling (please correct me here if
I'm wrong) that retrieving corresponding receipt may slow down the
execution that we tried to optimise.
BTW, maybe the data from `execution_outcome` is enough (we have there
`gas_burnt` and `tokens_burnt`). @jakmeier suggested the condition
`outcome.tokens_burnt == 0 && outcome.status == Success`. Unfortunately,
we need to work with any status. But maybe the first part
(`outcome.tokens_burnt == 0`) is enough to say that it could be only
refund receipt. I need more input here.
@telezhnaya telezhnaya mentioned this issue Oct 9, 2023
Merged
nikurt pushed a commit that referenced this issue Oct 11, 2023
@telezhnaya @nikurt
add status field to response in RPC methods tx, `EXPERIMENTAL_tx_st…
5692811 
…atus`, `broadcast_tx_commit`, `broadcast_tx_async` (#9556)

This PR is a part of RPC tx methods redesign
https://docs.google.com/document/d/1jGuXzBlMAGQENsUpy5x5Xd7huCLOd8k6tyMOEE41Rro/edit?usp=sharing
Addressing point 3 in #9542 and
start working on #6837

Changes:
- New field `status` appeared in the response of RPC methods `tx`,
`EXPERIMENTAL_tx_status`, `broadcast_tx_commit`, `broadcast_tx_async`
- Added some comments with the explanations

In #6837, Illia suggested to have
the structure
```rust
enum BroadcastWait {
 /// Returns right away.
 None,
 /// Waits only for inclusion into the block.
 Inclusion,
 /// Waits until block with inclusion is finalized.
 InclusionFinal,
 /// Waits until all non-refund receipts are executed.
 Executed,
 /// Waits until all non-refund receipts are executed and the last of the blocks is final.
 ExecutedFinal,
 /// Waits until everything has executed and is final.
 Final,
 }
```

I've renamed it to `TxExecutionStatus` and simplified it a little by
dropping all refund options:
1. We may add them later without breaking backwards compatibility
2. To support them, we need to have the access to `receipt` (it's the
method `get_tx_execution_status`). We have there only
`execution_outcome` by default. We created special logic to be able not
to retrieve the receipts (it's the only difference between `tx` and
`EXPERIMENTAL_tx_status`). I have a feeling (please correct me here if
I'm wrong) that retrieving corresponding receipt may slow down the
execution that we tried to optimise.
BTW, maybe the data from `execution_outcome` is enough (we have there
`gas_burnt` and `tokens_burnt`). @jakmeier suggested the condition
`outcome.tokens_burnt == 0 && outcome.status == Success`. Unfortunately,
we need to work with any status. But maybe the first part
(`outcome.tokens_burnt == 0`) is enough to say that it could be only
refund receipt. I need more input here.
github-merge-queue bot pushed a commit that referenced this issue Oct 12, 2023
@telezhnaya
jsonrpc: allow parse object at tx status requests (#9648)
22b7a36 
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.
@ilblackdragon
Copy link
Member Author

If implemented and used by wallets, this can speed up perceived transaction speed by 5-7 seconds in wallets.

telezhnaya added a commit to telezhnaya/nearcore that referenced this issue Oct 13, 2023
@telezhnaya
wip
e5b70cc 
near#6837
telezhnaya added a commit to telezhnaya/nearcore that referenced this issue Oct 13, 2023
@telezhnaya
wip
48989bb 
near#6837
telezhnaya added a commit to telezhnaya/nearcore that referenced this issue Oct 13, 2023
@telezhnaya
wip
3ee947f 
near#6837
nikurt pushed a commit that referenced this issue Oct 16, 2023
@telezhnaya @nikurt
jsonrpc: allow parse object at tx status requests (#9648)
87afd0a 
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.
@telezhnaya telezhnaya mentioned this issue Oct 16, 2023
Merged
github-merge-queue bot pushed a commit that referenced this issue Oct 24, 2023
@telezhnaya
jsonrpc: add finality to tx requests (tx, EXPERIMENTAL_tx_status)…
fa1cfa1 
…, add method `broadcast_tx` (#9644)

Fixes #6837

I'll need to update the [docs](https://github.com/near/docs) and
[jsonrpc_client](https://github.com/near/near-jsonrpc-client-rs) (and
maybe something else, I haven't already checked)

Important updates:
1. We have a new concept for all tx-related methods - `finality`.
`finality` in the response is merged to master 2 weeks ago:
#9556. `finality` field in the
request means "I want at least this level of confidence". So, the
stricter the level, the longer the user needs to wait.
2. I decided to use `Final` as a default `finality` value because it
gives the strongest guarantee and does not change backward compatibility
for any of the methods (though, the waiting time may be increased
sometimes to achieve the strictest level of confidence).
3. Methods `tx`, `EXPERIMENTAL_tx_status` have `finality` as an
additional optional field.
4. A new method appeared - `broadcast_tx`. It allows to send the tx, it
also have the optional field `finality`.
6. `broadcast_tx_async` is equal to `broadcast_tx` with hardcoded
`finality None`, I'll mark it as deprecated in the doc
7. `broadcast_tx_commit` is equal to `broadcast_tx` with hardcoded
`finality Final`, I'll mark it as deprecated in the doc.
nikurt pushed a commit that referenced this issue Nov 2, 2023
@telezhnaya @nikurt
jsonrpc: add finality to tx requests (tx, EXPERIMENTAL_tx_status)…
ad7ee12 
…, add method `broadcast_tx` (#9644)

Fixes #6837

I'll need to update the [docs](https://github.com/near/docs) and
[jsonrpc_client](https://github.com/near/near-jsonrpc-client-rs) (and
maybe something else, I haven't already checked)

Important updates:
1. We have a new concept for all tx-related methods - `finality`.
`finality` in the response is merged to master 2 weeks ago:
#9556. `finality` field in the
request means "I want at least this level of confidence". So, the
stricter the level, the longer the user needs to wait.
2. I decided to use `Final` as a default `finality` value because it
gives the strongest guarantee and does not change backward compatibility
for any of the methods (though, the waiting time may be increased
sometimes to achieve the strictest level of confidence).
3. Methods `tx`, `EXPERIMENTAL_tx_status` have `finality` as an
additional optional field.
4. A new method appeared - `broadcast_tx`. It allows to send the tx, it
also have the optional field `finality`.
6. `broadcast_tx_async` is equal to `broadcast_tx` with hardcoded
`finality None`, I'll mark it as deprecated in the doc
7. `broadcast_tx_commit` is equal to `broadcast_tx` with hardcoded
`finality Final`, I'll mark it as deprecated in the doc.
@jakmeier jakmeier mentioned this issue Mar 25, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@telezhnaya telezhnaya

Labels
A-RPC Area: rpc C-enhancement Category: An issue proposing an enhancement or a PR with one. T-integrations Team: issues relevant to the integrations team
Projects
None yet
Milestone
No milestone
5 participants
@ilblackdragon @telezhnaya @bowenwang1996 @austinabell @firatNEAR