Adding transaction_call endpoint #832
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #832 +/- ##
==========================================
- Coverage 60.95% 60.48% -0.48%
==========================================
Files 220 220
Lines 23425 23773 +348
==========================================
+ Hits 14278 14378 +100
- Misses 7982 8214 +232
- Partials 1165 1181 +16 ☔ View full report in Codecov by Sentry. |
|
This pull request has been marked as stale due to inactivity. It will be closed if no further activity occurs. Thank you for your contributions. |
MakisChristou
force-pushed
the
pedro/transactions_call
branch
from
6e7853e to
da734f1
Compare
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| } | ||
| } | ||
|
|
||
| return c.ResponseWriter.Write(b) |
Check warning
Code scanning / CodeQL
Reflected cross-site scripting Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix AI 2 days ago
To fix the reflected cross-site scripting vulnerability, we need to ensure that any user input is properly sanitized or escaped before being written to the HTTP response. In this case, we can use the html.EscapeString function from the html package to escape the user input before writing it to the response.
- We will modify the
Writemethod in thecallTxResponseWriterstruct to escape the user input before writing it to the response. - Specifically, we will escape the
VMErrorfield in therespstruct before writing the response.
| @@ -19,2 +19,3 @@ | ||
| "github.com/vechain/thor/v2/metrics" | ||
| "html" | ||
| ) | ||
| @@ -66,3 +67,3 @@ | ||
| if resp.VMError != "" { | ||
| c.VMError = resp.VMError | ||
| c.VMError = html.EscapeString(resp.VMError) | ||
| } |
| @@ -300,6 +301,25 @@ func TestClient_GetTransaction(t *testing.T) { | |||
| assert.Equal(t, expectedTx, tx) | |||
| } | |||
|
|
|||
| func TestClient_CallTransaction(t *testing.T) { | |||
There was a problem hiding this comment.
can we also add negative tests ?
There was a problem hiding this comment.
I guess we can but those are mock tests afaik. So not sure what the benefit of returning a hardcoded "error" response will be and checking if its what we expect.
| @@ -191,6 +191,177 @@ paths: | |||
| type: string | |||
| example: 'Invalid transaction ID' | |||
|
|
|||
| /transactions/call: | |||
There was a problem hiding this comment.
I haven't tried but when this yml gets generated, does it contain a working example ? (like the other endpoints)
|
|
||
| // Record VM error if present | ||
| if ctxWriter.VMError != "" { | ||
| metricTxCallVMErrors().AddWithLabel(1, map[string]string{ |
There was a problem hiding this comment.
What happens if there are a high number of different errors here ?
| if rt != nil && rt.GetName() != "" { | ||
| enabled = true | ||
| name = rt.GetName() | ||
| if name == "transactions_call_tx" { |
There was a problem hiding this comment.
What is the goal of this particular case ?
| @@ -74,6 +79,16 @@ func TestTransaction(t *testing.T) { | |||
| } { | |||
| t.Run(name, tt) | |||
| } | |||
|
|
|||
| // Call transaction | |||
| for name, tt := range map[string]func(*testing.T){ | |||
There was a problem hiding this comment.
Can another test be added here that does the same checks, or a summarized version, of the checks that the mempool does to verify tx integrity ?
There was a problem hiding this comment.
Ah! These seem to be covered already yeah ?
Description
This PR adds a new endpoint that allows to simulate and estimate based on transactions instead of clauses.
The added value is to bring api communication to the transaction level. This allows for gotchas like, chaintag, expiration, nonce, etc to be more visible and allows executing transactions in a given block revision
Summary of the endpoint:
Goal: Execute a transaction at the head of a specified block. Retrieve the total gas cost at the transaction level.
(Note: Can be expanded to execute in a block at a given tx position.)
Fixes # (issue)
Type of change
How Has This Been Tested?
Checklist: