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.

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

Algod: Minor refactoring REST client submitForm from go-sdk PR #335 #5253

Merged
merged 5 commits into from
Apr 5, 2023

Conversation

ahangsu
Copy link
Contributor

@ahangsu ahangsu commented Apr 3, 2023

Summary

The motivation of this PR is that, we want to allow for algod to simulate with option of unlimit-log: see #5230.

We need to send transaction with auxiliary query parameters. The current implementation in restClient disallowed us for such feasibility, while algorand/go-algorand-sdk#335 contain relevant modification, and this PR mimic the changes, by separating request into params and body.

We expect to use this for simulation, where /v2/transactions/simulate has query parameter unlimit-log: bool, with raw body of txn(-group). The simulation request body resembles the one in compilation, where /v2/teal/compile has query parameter sourcemap: bool and raw body of program source bytes.

Test Plan

Implement Compile with sourcemap option, and check existence of sourcemap field, which can be controlled by request option Sourcemap *bool.

@ahangsu ahangsu force-pushed the refactoring-algod-submit-form branch from c39af7e to cdabcf7 Compare April 3, 2023 19:57
@codecov
Copy link

codecov bot commented Apr 3, 2023

Codecov Report

Merging #5253 (e788b1d) into master (7703bc4) will decrease coverage by 0.02%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           master    #5253      +/-   ##
==========================================
- Coverage   53.76%   53.74%   -0.02%     
==========================================
  Files         450      450              
  Lines       56184    56184              
==========================================
- Hits        30206    30195      -11     
- Misses      23628    23639      +11     
  Partials     2350     2350              
Impacted Files Coverage Δ
data/transactions/logic/sourcemap.go 100.00% <ø> (ø)
libgoal/teal.go 0.00% <0.00%> (ø)

... and 5 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@ahangsu ahangsu marked this pull request as ready for review April 3, 2023 20:27
@ahangsu ahangsu changed the title Algod: Minor refactoring rest client submitForm from go-sdk PR #335 Algod: Minor refactoring REST client submitForm from go-sdk PR #335 Apr 3, 2023
Copy link
Contributor

@jasonpaulos jasonpaulos left a comment

Choose a reason for hiding this comment

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

Generally looks good. This is a problem I ran into while working on simulate earlier.

In fact I made this follow-up PR to improve simulation response decoding by requesting msgpack responses: ahangsu#1. I can open a PR in go-algorand for that after this one is merged

daemon/algod/api/client/restClient.go Outdated Show resolved Hide resolved
@ahangsu ahangsu requested a review from jasonpaulos April 4, 2023 17:17
jasonpaulos
jasonpaulos previously approved these changes Apr 4, 2023
Co-authored-by: Bob Broderick <118225939+bbroder-algo@users.noreply.github.com>
@ahangsu ahangsu force-pushed the refactoring-algod-submit-form branch from 45d5225 to 489da67 Compare April 5, 2023 15:42
@bbroder-algo bbroder-algo merged commit f4f5ec6 into algorand:master Apr 5, 2023
@ahangsu ahangsu deleted the refactoring-algod-submit-form branch April 7, 2023 18:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants