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

Assert that Atomic Transaction Composer's method adder checks for arg-length consistency #190

Open
tzaffi opened this issue Jun 14, 2022 · 0 comments
Labels
new-feature-request Feature request that needs triage Team Scytale

Comments

@tzaffi
Copy link
Contributor

tzaffi commented Jun 14, 2022

Problem (As of June 14, 2022)

graviton PR #21 aims to give confidence that PyTeal's soon-to-be relased ABI-Router works as expected. That work revealed that the ABI-Router's generated TEAL code, does not assert that exactly the number of arguments expected to be passed into an ABI-method call, are actually passed. A discussion around this issue resulted in a consensus around what actions to take. It was decided that since the vast majority of PyTEAL users are likely to use an Algorand supported SDK for interacting with the associated smart contract, it is sufficient that the client code assert this condition.

After further investigation, it appears that all 4 SDK's do indeed ensure that the number of arguments passed in a method call during execution, are the number that are defined in accordance to the method's specification.

However, the PROBLEM is that this is not officially tested for via Cucumber.

Solution

Introduce a new scenario with new steps that make this assertion.

Example Implementation

Python SDK PR #347 is one such example.

Dependencies

None

Urgency

Medium - as we want to have maximum confidence at the time of PyTEAL's ABI-Router release that the process of creating and using associated smart contract is as least error prone as possible.

Background / Notes for SDK Developers

Upshot Universal Error Message Regex

incorrect number of arguments|incorrect method arg number|incorrect number of method arguments|number of method arguments do not match the method signature

↠ (excepting java)
number.*arguments

Summary of all Error Messages:

  • go
    • the incorrect number of arguments were provided …
  • java
    • Method call error: incorrect method arg number provided
  • javascript
    • Incorrect number of method arguments. Expected …
  • python
    • number of method arguments do not match the method signature

go

“the incorrect number of arguments were provided”

func (atc *AtomicTransactionComposer) AddMethodCall(params AddMethodCallParams) error
future/atomicTransactionComposer.go

if len(params.MethodArgs) != len(params.Method.Args) {
        return fmt.Errorf(“the incorrect number of arguments were provided: %d != %d”, len(params.MethodArgs), len(params.Method.Args))
    }

Java

“Method call error: incorrect method arg number provided”

class MethodCallParams::MethodCallParams()

src/main/java/com/algorand/algosdk/transaction/MethodCallParams.java

if (method.args.size() != methodArgs.size())
	throw new IllegalArgumentException("Method call error: incorrect method arg number provided");

JavaScript

“Incorrect number of method arguments. Expected ${method.args.length}, got ${methodArgs.length}”

export class AtomicTransactionComposer::addMethodCall():
src/composer.ts

    if (methodArgs.length !== method.args.length) {
      throw new Error(
        `Incorrect number of method arguments. Expected ${method.args.length}, got ${methodArgs.length}`
      );
    }

python

“number of method arguments do not match the method signature”

class AtomicTransactionComposer::add_method_call()
algosdk/atomic_transaction_composer.py

   if len(method.args) != len(method_args):
       raise error.AtomicTransactionComposerError(
"number of method arguments do not match the  method signature")
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new-feature-request Feature request that needs triage Team Scytale
Projects
None yet
Development

No branches or pull requests

1 participant