Problem

As of algorand/go-algorand#3890, accessing a Box requires specifying the box name in the (new) static Boxes array. The story requests adding SDK support + tests for submitting Application Call transactions with a static Boxes array.

SDKs in-scope:

• https://github.com/algorand/go-algorand-sdk/
• https://github.com/algorand/java-algorand-sdk
• https://github.com/algorand/js-algorand-sdk
• https://github.com/algorand/py-algorand-sdk

Solution

• We should able to specify the boxes that an application call will touch: i.e. ApplicationCallTxn needs a new "boxes" field.
• The atomic transaction composer also needs an update to the add_method_call function, and possibly some other updates (not well understood by me yet).
• Encoding note:
  • The SDK exposes an API where apps are directly referenced: appl(A, ForeignApps=[B], Boxes=[(A, "hello"), (B, "goodbye")])
  • In contrast, the transaction encoding translates the app reference to the foreign app index (e.g. A to 0): appl(A, ForeignApps=[B], Boxes=[(0, "hello"), (1, "goodbye")]).

Dependencies

Urgency

currently this e2e test only tests algod, should also test that the situation behaves correctly on indexer.

but make the indexer part of the test should be optional.

Problem

We want to test C2C inner transaction call described in c2c-feature-branch, and the discussion/design-doc is here.

We might want to have some deep-calls (a chain of inner-trans call) as testing method.

Problem

Merging algorand-sdk-testing in a state that breaks downstream SDKs in master shouldn’t happen. But, in practice, it happens without mal-intent.

Background:

• Since there’s no build check in algorand-sdk-testing, it’s possible to unintentionally merge test changes that break SDKs.
• Since there’s no time-based SDK build process (e.g. nightly), no one discovers a broken SDK build until someone tries to build the project. It’s difficult to correlate a build failure with root problem cause.
• Motivating example:
  • Unintentionally, #162 was merged without corresponding JS and Python SDK changes.
  • Post-merge, folks surfaced the problem through community contribution PRs (e.g. algorand/py-algorand-sdk#271).
  • After surfacing the problem, the gap was closed:
    • algorand/js-algorand-sdk#540
    • algorand/py-algorand-sdk#301
• With the as is conventions, the encouraged way to make changes without breaking SDKs is to make 2-step changes as described below.
  • In an algorand-sdk-testing PR, duplicate the test in-question. Assign a unique annotation.
  • Merge the PR. SDKs are safe because:
    • New test is not run until the annotation is configured.
    • All other tests are unchanged.
  • After making + merging changes to all SDKs, remove the test that was duplicated.

Solution

Unclear - From group discussion, at least a couple of paths exist. Bolded proposal is preferred though more discussion is needed.

• Modify algorand-sdk-testing build process to test downstream SDKs. Implies failing the build when a downstream SDK's tests fail.
  • There's a circular dependency between algorand-sdk-testing and SDKs that complicates the checks.
  • It's possible to imagine a build process like:
    • algorand-sdk-testing build looks for SDK branch names matching algorand-sdk-testing branch. If none found, use the SDK repo's default branch.
    • Run integration tests on all SDKs.
  • Concerns:
    • Some algorand-sdk-testing changes have no downstream consequences. The process must work for such scenarios.
    • The proposed process feels heavyweight. It likely results in a long build time.
• Make SDKs depend on a tag of algorand-sdk-testing instead of master. Implies implementing a process to ensure SDK repos update algorand-sdk-testing on release.
  • Allows for looser coupling among SDK changes. Should support retaining a simpler build process.
  • It's possible to imagine starting with a manual process for keeping tags in-sync. In principle, stories won't be closed until corresponding SDK changes complete.
  • Constraint: All SDKs provide the same approach for configuring algorand-sdk-testing git tag/branch without requiring hand-modified shell script modifications.

Stopgap measures until taking on an approach from above:

• Setup a nightly build for each SDK.
  • If an algorand-sdk-testing PR breaks an SDK, we'll know within 24 hours rather than when someone else attempts to build.
  • The process feels straightforward to implement. And provides more visibility than we have today.
  • Once a more principled approach is implemented, we can remove the nightly builds.

Dependencies

N/A

Urgency

TBD

@winder commented on Thu Jul 23 2020

• Indexer responses which have transactions.
• Algod PendingTransactionByID (maybe - only Java supports comparing messagepack responses against the model objects)
• Regenerate indexer integration test goldens (utils/create_indexer_integration_goldens.sh)

Currently it is difficult for a new developer to setup, run tests locally, and add new tests. For example, each step-definition file uses lots of global state to pass information around. This was fine when we had only a few tests, but as time passes and the feature and test sets grow, this will get harder and harder to maintain.

Some suggestions:

• refactor stepdefs to use a test fixture that resets per-scenario
• setup and test scripts do not need to be edited before running
• ??? what else?

Additional notes and ideas in this document:
https://algorand.atlassian.net/wiki/spaces/FDE/pages/487358465/SDK+Improvement+Ideas

Problem

algorand/go-algorand#1654 adds an algod API endpoint to disassemble TEAL bytecode. The ticket requests updating all downstream SDKs to support the endpoint.

Solution

Dependencies

N/A

Urgency

TBD

Problem

Make corresponding SDK changes to support these Box search APIs:
algorand/indexer#978.

This story is the Indexer analog to #175.

SDKs in-scope:

• https://github.com/algorand/go-algorand-sdk/
• https://github.com/algorand/java-algorand-sdk
• https://github.com/algorand/js-algorand-sdk
• https://github.com/algorand/py-algorand-sdk

Solution

Tracking PR progress:

Dependencies

Urgency

Yesterday the Javascript SDK started failing an integration test: https://travis-ci.com/github/algorand/js-algorand-sdk/jobs/403990024

The only difference between the actual and expected JSON is that some accounts have a nonzero "reward", so I believe this is because algorand/indexer#221 was merged.

I was going to make a PR that hardcodes the values from the actual response above, but since rewards are time dependent I wasn't sure if that would be too brittle.

Subject of the issue

Your environment

Steps to reproduce

Expected behaviour

Actual behaviour

We need feature files for the SDKs to implement indexer REST integration tests.

Currently, port and token values are received from the environment, or hardcoded. It would likely be better to define these in the .feature files.

Example from indexer testing:

  Background:
    Given indexer client 1 at "localhost" port 59999 with token "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"

Application Transaction Goldens

• Paths
• Responses

Subject of the issue

Various attempts were made to fix algorand/js-algorand-sdk#414 but the PRs were blocked by failed cucumber tests which have an inaccurate expected behaviour for the feature.

See below

Expected behaviour

These are the expected behaviour for the SDK.

1. [working as expected] When currencyGreaterThan or currencyLessThan is unset, it should not appear in the url query list.
2. [working as expected] When currencyLessThan is set to 0, the returned results should be empty or throw an error for disallowed value.
3. [working as expected] When currencyGreaterThan is set to 0, the returned result from the API is different from that of when currencyGreaterThan is unset.
   => And that's why currency-greater-than=0 should be in the query list if 0 is set explicitly.
4. [not working] When currencyGreaterThan or currencyLessThan is set to 0, it should appear in the url query list.

Actual behaviour

There are many tests in https://github.com/algorand/algorand-sdk-testing/blob/master/features/unit/v2indexerclient_paths.feature that has the same inaccurate expectation. Below is one of the examples:
currency-greater-than=0 or currency-less-than=0 is expected to not appear in the url's query list by the cucumber tests even though 0 is being explicitly set.

Rekey an address, create and sign transaction with the new key, submit and validate

Need to create new indexer test data and the new rekeying tests

Problem

https://github.com/algorandfoundation/ARCs/blob/main/ARCs/arc-0004.md defines the following contract name validation rules:

Contract names MUST satisfy the regular expression [A-Za-z][A-Za-z0-9]*

SDKs allow any string to be used in contract creation. The story requests enforcing ARC-4 contract name validation rules.

Solution

Dependencies

Urgency

• Scenario: Split steps implemented
• Scenario: HTLC steps implemented
• Scenario: Periodic Payment steps implemented
• Scenario: Limit Order steps implemented
• Scenario: Dynamic Fee steps implemented
• Tests actually pass

• Integration Tests

Test support for SDKs invoking on-chain contracts with the ABI. Specifically, test the implementation of the AtomicTransactionComposer (the second feature) from this design document for each SDK.

This likely requires a new integration test feature file.

• Scenario: Split steps implemented
• Scenario: HTLC steps implemented
• Scenario: Periodic Payment steps implemented
• Scenario: Limit Order steps implemented
• Scenario: Dynamic Fee steps implemented
• Tests actually pass

The test environment is currently hard coded to install the nightly channel.

We need an option that allow building a specific go-algorand branch / repository to use with the test environment.

Context:

currently, algod supports a REST endpoint that returns proof of a specific transaction inside a block.
The new statproofs keys introduce merkle tree changes that imply changes on the proof that is being returned.
Thus, the REST endpoint is going to change

Acceptance Criteria :

go-algorand-sdk should support this API changes.

Relates to #18

Change url to use S3 per Tsachi's update algorand/go-algorand#670

• Scenario: Split steps implemented
• Scenario: HTLC steps implemented
• Scenario: Periodic Payment steps implemented
• Scenario: Limit Order steps implemented
• Scenario: Dynamic Fee steps implemented
• Tests actually pass

Problem

Solution

Dependencies

Urgency

Add additional tests for ABI, including tests for foreign object type arguments and changes from algorandfoundation/ARCs#53

• Scenario: Split steps implemented
• Scenario: HTLC steps implemented
• Scenario: Periodic Payment steps implemented
• Scenario: Limit Order steps implemented
• Scenario: Dynamic Fee steps implemented
• Tests actually pass

-update docker and scripts
-tweak feature files and goldens where needed (see open PR #65)
-fix bugs in sdks exposed by tests
-report bugs outside sdks exposed by tests
-fix regression in v1 healthcheck
-keep SDKs updated per changes to indexer/algodv2 changes

Setup a containerized environment to make it easier to run tests locally. Needs to allow injecting local SDK code for testing.

Per 11-Dec message in team-forward-deployed

• Java
• Python
• Go
• Javascript

Introduction

algorand/go-algorand#3287 has introduced min-balance to the response of algod's /v2/accounts/{{ACCOUNT}} as well as to the goal account info. "For free", it is also introduced into the Python SDK's account_info() method. We'll need to implement this functionality in all our other SDK's. To ensure that the SDK's behave as expected, we should adhere to a spec that is defined in a cucumber test.

part of: algorand/indexer#808

Output

1. Cucumber test that describes what the min-balance field should look like when calling account_info()
2. Issues for implementing the feature and passing tests in each of the 4 SDK's

Problem

When supplying an app in the foreign app's array, the app's account is inaccessible unless it's added to the foreign accounts array. The workflow causes SDK users to stumble over dryrun usage because the requirement feels non-obvious.

Solution

When composing a dryrun request, SDKs can conditionally add the app's account to foreign accounts arrays.

Dependencies

Urgency

Test support for SDKs invoking on-chain contracts with the ABI. Specifically, test the implementation of JSON description objects (the first feature) from this design document for each SDK.

This likely requires a new unit test feature file.

We had to create a couple new steps to add parameters to existing steps without breaking old stuff:

  @unit.indexer.rekey
  Scenario Outline: LookupAssetTransactions path for rekey
    When we make a Lookup Asset Transactions call against asset index <index> with NotePrefix "<notePrefixB64>" TxType "<txType>" SigType "<sigType>" txid "<txid>" round <round> minRound <minRound> maxRound <maxRound> limit <limit> beforeTime "<beforeTime>" afterTime "<afterTime>" currencyGreaterThan <currencyGreaterThan> currencyLessThan <currencyLessThan> address "<address>" addressRole "<addressRole>" ExcluseCloseTo "<excludeCloseTo>" RekeyTo "<rekeyTo>"

When we make a Lookup Asset Transactions... now has an implementation with/without RekeyTo "<rekeyTo>".

We could migrate the original LookupAssetTransactions path for rekey to the new implementation, and tag the old one with @deprecated. In the future once all of our SDKs have implemented the new step we can delete the old one and remove @unit.indexer.rekey

Repeat this for all of the new "call endpoint with parameters" steps.

When we make a Lookup Asset Transactions call against asset index <index> with NotePrefix "<notePrefixB64>" TxType "<txType>" SigType "<sigType>" txid "<txid>" round <round> minRound <minRound> maxRound <maxRound> limit <limit> beforeTime "<beforeTime>" afterTime "<afterTime>" currencyGreaterThan <currencyGreaterThan> currencyLessThan <currencyLessThan> address "<address>" addressRole "<addressRole>" ExcluseCloseTo "<excludeCloseTo>" RekeyTo "<rekeyTo>"

When we make a Lookup Account Transactions call against account "<account>" with NotePrefix "<notePrefixB64>" TxType "<txType>" SigType "<sigType>" txid "<txid>" round <round> minRound <minRound> maxRound <maxRound> limit <limit> beforeTime "<beforeTime>" afterTime "<afterTime>" currencyGreaterThan <currencyGreaterThan> currencyLessThan <currencyLessThan> assetIndex <index> rekeyTo "<rekeyTo>"

When we make a Search Accounts call with assetID <index> limit <limit> currencyGreaterThan <currencyGreaterThan> currencyLessThan <currencyLessThan> round <round> and authenticating address "<authAddr>"

When we make a Search For Transactions call with account "<account>" NotePrefix "<notePrefixB64>" TxType "<txType>" SigType "<sigType>" txid "<txid>" round <round> minRound <minRound> maxRound <maxRound> limit <limit> beforeTime "<beforeTime>" afterTime "<afterTime>" currencyGreaterThan <currencyGreaterThan> currencyLessThan <currencyLessThan> assetIndex <index> addressRole "<addressRole>" ExcluseCloseTo "<excludeCloseTo>" rekeyTo "<rekeyTo>"

All the response checks in algodv2 and indexer _response feature files look like:
And the parsed Pending Transactions Information response should contain an array of len <len> and element number <idx> should have sender "<sender>"

Instead, they could be:
Then the re-serialized object equals the response in "<jsonfiles>" loaded from "<directory>"
This would let the tests check the whole struct rather than individual fields, would make tests cleaner to read, easier to write, and more thorough.

#54

4/16 update: this includes additional found work for generating the test data

We should add some additional metadata to each SDK to allow the harness to do some basic validation prior to running the tests. At the root of each directory we could have a file named harness.properties that provides some metadata to the SDK Harness.

Another thing we could do is move the step files into each SDK, and then the property file would help the cucumber harness find the step files.

Just forwarding a recommendation I got to use data tables:

cucumber-rs/cucumber#183 (comment)

Instead

And I build an application transaction with the transient account, the current application, suggested params, operation "create", approval-program "programs/big_app_program.teal.tok", clear-program "programs/big_app_program.teal.tok", global-bytes <global-bytes>, global-ints 0, local-bytes <local-bytes>, local-ints 0, app-args "", foreign-apps "", foreign-assets "", app-accounts "", extra-pages 3

do (or similar)

And I build an application transaction with 
  | operation        | create                            |
  | approval-program | programs/big_app_program.teal.tok |
  | clear-program    | programs/big_app_program.teal.tok |
  | global-bytes     | <global-bytes>                    |
  | global-ints      | 0                                 |
  | local-bytes      | <local-bytes>                     |
  | local-ints       | 0                                 |
  | app-args         |                                   |
  | foreign-apps     |                                   |
  | foreign-assets   |                                   |
  | app-accounts     |                                   |
  | extra-pages      | 3                                 |

It would be easier both read/match the step and access the table data via gherkin::Step.

I imagine that these kind of changes are difficult to do, since they'd break the tests in all the SDKs, but worth maybe for future features.

Problem

For each SDK I've written a small utility method to the effect of:

def get_method(c: Contract, name: str) -> Method:
    for m in c.methods:
        if m.name == name:
            return m
    raise Exception("No method with the name {}".format(name))

So that later I can call:

comp.add_method_call(app_id, get_method("add"), addr, sp, signer, method_args=[1,1])

Solution

Implement this method or something like it so a developer can easily pass the contract Method into the add_method_call without having to rewrite the same utility function.

Sometimes, on Travis and on local machines, the SDK tests fail with some kind of file error / the error ENOENT also appears in many places. This can typically be fixed with a retry.

Will update with a more precise description and a log when I can.

Problem

When application developers debug a dryrun, it's helpful to format the response in a trace-like format.

Solution

Dependencies

N/A

Urgency

TBD

Update SDK harness to include indexer instance(s) preloaded with static test data.