-
Notifications
You must be signed in to change notification settings - Fork 6
Conversation
Hi Nat, this is good thank you for submitting. I was actually working on the same based on your medium post, and not as far along. I am taking a look now. |
one ask, can we better align with cleos send_transaction calling patterns? Specifically my ask is to include useOldSendRPC flag to transact. We can add a retry_irreversible later. |
Hm looks like the wires got crossed there. Perhaps you can do a diff check with https://www.diffchecker.com/ and share the link for the various files you see being changed. Yes I agree it should match cleos. In the meantime I'll try updating what parts I can piece together. |
…ble, add assertion, pass retry blocks if not irreversible, make num blocks optional param
Here is the diff |
Okay great, I added that as well as the irreversible feature, let me know what your thoughts are. I also update the PR notes at the top to reflect removing the I initially made the example in the README to use irreversible blocking by default, but in the days of dfuse 3 handoffs was considered pretty guaranteed. So perhaps Also to consider is that the default maximum amount of blocks for the nodeos to allow to retry is 90 blocks, so irreversible trx retries will cause the following error:
|
Exciting stuff, thanks for the quick turnaround. I did a quick check and it looks good. I want to go back and closer look at irreversible updates in cleos code for comparison. |
I've added a change to not use the retry feature by default and to return the failure trace by default to also match cleos.
Also some community feedback from Aaron of Greymass: https://t.me/c/1139062279/312536 |
@@ -85,6 +85,51 @@ const transactWithoutBroadcast = async () => await api.transact({ | |||
blocksBehind: 3, | |||
expireSeconds: 30, | |||
}); | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll add in a test for the old_send_rpc path. These new tests are good, thanks for adding them.
@@ -105,6 +105,14 @@ export interface GetInfoResult { | |||
virtual_block_net_limit: number; | |||
block_cpu_limit: number; | |||
block_net_limit: number; | |||
server_version_string: string; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is needed , thanks
} | ||
if(retryTrxNumBlocks > 0) { | ||
params = { | ||
...params, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
irreversible well done
Change Description
Support Issue #5
Related: eosnetworkfoundation/product#30
/v1/chain/send_transaction2
./v1/chain/send_transaction2
.text-encoding
as dev dependency in package.json to run unit tests.server_version_string
belowv3.1.0
GetInfoResult
type with latest returned resultsretry transaction
fornode.test.ts
API Changes
returnFailureTrace
option, default falseretryIrreversible
option, default falseretryTrxNumBlocks
option, default 0useOldRPC
option, default falseuseOldSendRPC
option, default falseDocumentation Additions
README.md
file for 5 new API parameters.