fix: use named parameter check instead of fragile >= 6 count for ABI version detection

[ygd58]

Summary

Replaces a fragile argument-count check with a named-parameter check for detecting the addTransaction ABI version.

Problem

if len(inputs) >= 6:
    return "v6"

This check works accidentally today (Bradbury has 6 args, Asimov has 5), but will silently break if a future protocol upgrade adds a 7th argument — causing an extra positional argument to be supplied and corrupting transaction encoding.

Fixes #310

Fix

input_names = [inp.get("name", "") for inp in inputs]
if "valid_until" in input_names:
    return "v6"

This correctly detects the v6 ABI by checking for the presence of the valid_until named parameter, regardless of how many other arguments are added in future upgrades.

Summary by CodeRabbit

• Bug Fixes
  • Improved version detection for transaction submission so the app now more reliably handles different request formats.
  • Better distinguishes between supported transaction input structures, reducing the chance of version-related errors.

[github-actions]

This PR targeted main, which is only the default/static branch.

I retargeted it to v0.19-dev, the active development branch. Pushes to v0.19-dev automatically fast-forward main.

[coderabbitai]

📝 Walkthrough

Walkthrough

The _get_add_transaction_abi_version function in genlayer_py/contracts/actions.py was modified to determine the ABI version of addTransaction by checking for the presence of a valid_until parameter name in the ABI inputs, replacing the previous logic that relied on counting input parameters.

Changes

ABI Version Detection

Layer / File(s)	Summary
Name-based version check genlayer_py/contracts/actions.py	Replaces input-count check with name-based detection of valid_until to determine addTransaction ABI version (v5 vs v6).

Estimated code review effort: 1 (Trivial) | ~5 minutes

Possibly related issues

• [Bug] _encode_add_transaction_data uses fragile >= 6 check to detect ABI version, will break with ABI updates genlayer-cli#310: Addresses the same addTransaction ABI-version detection logic, replacing the argument-count check with a valid_until name check.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the ABI version detection fix and the switch from input-count logic to a named-parameter check.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

🧹 Nitpick comments (1)

genlayer_py/contracts/actions.py (1)

604-606: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add a test case for the name-based detection edge case.

The existing tests (test_encode_add_transaction_uses_v5_signature_when_abi_has_5_inputs and ..._v6_..._when_abi_has_6_inputs) cover the standard 5-input/v5 and 6-input/v6 ABIs, but they don't validate the specific scenario this PR fixes: an ABI with 6+ inputs that does not contain valid_until should be detected as v5. Adding such a test would confirm the new name-based logic behaves differently from the old count-based logic.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@genlayer_py/contracts/actions.py` around lines 604 - 606, Add a test for the
name-based version detection edge case in the transaction encoding tests around
encode_add_transaction and the signature selection helper: cover an ABI with 6
or more inputs that does not include valid_until and assert it is detected as
v5, not v6. Update the existing test suite near
test_encode_add_transaction_uses_v5_signature_when_abi_has_5_inputs and
test_encode_add_transaction_uses_v6_signature_when_abi_has_6_inputs to include
this scenario so the new input-name logic is exercised instead of the old
count-based behavior.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@genlayer_py/contracts/actions.py`:
- Around line 604-606: Add a test for the name-based version detection edge case
in the transaction encoding tests around encode_add_transaction and the
signature selection helper: cover an ABI with 6 or more inputs that does not
include valid_until and assert it is detected as v5, not v6. Update the existing
test suite near
test_encode_add_transaction_uses_v5_signature_when_abi_has_5_inputs and
test_encode_add_transaction_uses_v6_signature_when_abi_has_6_inputs to include
this scenario so the new input-name logic is exercised instead of the old
count-based behavior.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 2be5287e-31ee-4ee8-a37c-8e0dcd7429ff

📥 Commits

Reviewing files that changed from the base of the PR and between dd25ef7 and aa49fc3.

📒 Files selected for processing (1)

• genlayer_py/contracts/actions.py