java-algorand-sdk icon indicating copy to clipboard operation
java-algorand-sdk copied to clipboard

Expose `min-balance` field in **Account Info**

Open tzaffi opened this issue 4 years ago • 8 comments

Problem

min-balance is available in algod but not in the Java SDK.

Solution

https://github.com/algorand/go-algorand/pull/3287 has introduced min-balance to the response of algod's /v2/accounts/{{ACCOUNT}} as well as to the goal account info.

  • When querying the REST API avove, expose the min-balance field
  • Pass appropriate cucumber tests

Dependencies

  • Cucumber tests issue: https://github.com/algorand/algorand-sdk-testing/issues/155

Urgency

Low

tzaffi avatar Dec 17 '21 16:12 tzaffi

Is it okay if I can take a stab at this?

I am wondering how to effectively update the sdk-testing repo and the individual repos for cucumber test changes. Is the idea is to push the changes to all repo at once?

  1. If we change cucumber test in algorand-sdk-testing to include the new parameter then we have to change the signature of the function in all the language specific sdks or the tests will start failing due to signature mismatch.

https://github.com/algorand/algorand-sdk-testing/blob/c306203d1f7bb2f1c4cc90c96256c9db60de6504/features/integration/indexer.feature#L147-L150

  1. The reverse is also true. We cannot update the language specific sdk integration test without updating the algorand-sdk-testing repo.

https://github.com/algorand/java-algorand-sdk/blob/e9871a544ad6155df8cb67f5d95ea38254f8a6a7/src/test/java/com/algorand/algosdk/integration/Indexer.java#L198-L200

ghost avatar Mar 09 '22 04:03 ghost

That's a good point. You can add a new cucumber test together with a filter label (something like @accounts.min-balance) and then only run the test in the SDK that you have implemented. In that way, you won't break any of the other SDK's.

tzaffi avatar Mar 09 '22 14:03 tzaffi

@tzaffi Thank you!

I am not working on this anymore.

ghost avatar Mar 11 '22 01:03 ghost

Hey @tzaffi , quick question regarding this. When we refer to the /v2/accounts/{{ACCOUNT}} REST call above which are we referring to ?

/v2/accounts/{address} - algod /v2/accounts/{account-id} - indexer

Based on what you've said above, as well as having looked at the swaggers I believe the change needs to be made for algod, however the person who last worked on this has suggested making changes in the Indexer response

jds1g14 avatar Apr 28 '22 20:04 jds1g14

hi @jds1g14 - this issue when it was created was to provide min-balance functionality in the Java SDK, via algod. However, I expect that in a couple of weeks, indexer will be providing min-balance as well. Depending on the timing of when this issue is addressed, it may be possible to address the indexer functionality in the Java SDK as well. But technically, it's outside the scope of this issue. I Would probably create new issues when min-balance becomes available for indexer.

tzaffi avatar Apr 28 '22 20:04 tzaffi

Perfect, I'll start work on this.

  1. Create/modify any existing tests and response files in algorand-sdk-testing, adding an appropriate tag
  2. Update model in java-algorand-sdk

jds1g14 avatar Apr 28 '22 22:04 jds1g14

I wonder why this isn't autogenerated. It appears to be in the OpenAPI spec: https://github.com/algorand/go-algorand/blob/master/daemon/algod/api/algod.oas2.json#L2466

winder avatar Apr 11 '23 20:04 winder

It's because Account is shared between the OpenAPI spec in go-algorand and Indexer, but the Indexer version is used to generate the model types. The Indexer spec doesn't include min-balance because that feature was never added to Indexer.

winder avatar Apr 12 '23 00:04 winder