fix(taiko-client): run the integration tests against the PR's own protocol

[dantaik]

The bug

taiko-client--test.yml ran its integration tests against contracts from a second checkout pinned to main, not the contracts in the pull request:

- name: Checkout protocol for testing
  uses: actions/checkout@v7
  with:
    repository: taikoxyz/taiko-mono
    path: ${{ env.PROTOCOL_FORK_DIR }}
    ref: main                                    # <-- not the PR
...
  PROTOCOL_DIR: ${{ github.workspace }}/${{ env.PROTOCOL_FORK_DIR }}/packages/protocol

Two consequences. A protocol change could never be validated against the client in the same PR — it only surfaced after merging, in some unrelated PR's CI. And because main moves, the job tested different contracts on every re-run, so a green result did not mean the same thing twice.

taiko-client-rs--test.yml already deploys the PR's own packages/protocol, with a comment giving exactly this reasoning. This brings the Go workflow in line.

What changed

• taiko-client--test.yml — drop the main-pinned checkout and PROTOCOL_FORK_DIR; point PROTOCOL_DIR at ${{ github.workspace }}/packages/protocol, which the job already has checked out.
• Both workflows — trigger on packages/protocol/contracts/** and packages/protocol/script/layer1/core/**. These are what the tests deploy through DeployProtocolOnL1, so a change to them can break the client without touching a single client file. Without this the fix above would still never fire on a protocol-only change.
• bindings/encoding/input_test.go (new) — boundary coverage for basefeeSharingPctg.

basefeeSharingPctg at 0 and 100

The inbox accepts 0 <= basefeeSharingPctg <= 100 (LibInboxSetup.validateConfig), and taiko-geth splits each transaction's basefee as gasUsed * baseFee * pctg / 100 to the coinbase with the remainder to the treasury. So 0 pays the coinbase nothing and 100 pays the treasury nothing — both ends are reachable configurations, not edge cases.

The client itself is already indifferent: it only plumbs the byte into extraData and never does arithmetic with it. The exposure was entirely in tests that assumed a split. EncodeShastaExtraData had no test at all, so this adds three:

• every value in {0, 1, 25, 75, 99, 100} round-trips through core.DecodeShastaBasefeeSharingPctg;
• the percentage and the proposal ID never read each other's bytes, including when either is zero, up to the maximum uint48 proposal ID;
• the three proposal IDs the encoder refuses stay refused, so an out-of-range ID can't truncate into the percentage byte.

Interaction with #22127

#22127 moves DevnetInbox to basefeeSharingPctg: 100 and makes the two treasury integration tests derive their expectation from the inbox config rather than assuming the treasury always gains. The two PRs compose in either merge order: on main today the devnet inbox is still 75, so these tests pass unchanged here; once #22127 lands, the integration suite deploys and exercises 100 end to end for the first time.

Testing

go test ./bindings/encoding/ and go vet ./bindings/encoding/ pass locally; both workflow files parse as YAML. The integration lanes on this PR are the real check for the workflow change — they should now deploy this branch's contracts rather than main's.

🤖 Generated with Claude Code

https://claude.ai/code/session_01UhqmeYDadT95CmcUhWzc2B

---

Generated by Claude Code

[dantaik]

Took four of the five findings, pushed in f5f3e40.

Warning 1 — path filters narrower than what the tests consume. Correct, and worse than reported.

Traced the closure: DeployProtocolOnL1 imports from the test tree as well — test/shared/DeployCapability.sol, test/shared/helpers/FreeMintERC20Token*.sol and test/layer1/core/inbox/mocks/MockContracts.sol — none of which contracts/** or script/layer1/core/** covered.

Rather than extend the list to script/layer1/** as suggested, I dropped the enumeration. Any hand-maintained list goes stale the moment someone adds an import, which is the same silent gap this PR exists to close — and it fails invisibly. The filter is now protocol.yml's own path set: whatever can affect the protocol build can affect the contracts these tests deploy. deployments/** stays excluded because deploy_l1.json is gitignored output written by the run, not read into it.

The cost is real and worth stating: the client integration lanes will now run on most protocol PRs. That is a deliberate trade of CI minutes for a failure mode that is loud instead of silent — happy to narrow it if the team would rather not pay that.

Suggestion — basefeeSharingPctgRange claims more than it covers. Fair; fixed by widening the coverage, not the wording. It now returns all of 0..100, so the name and comment are literally true, and the test runs in 0.03s.

Suggestion — the truncated comment. Fixed. "at 100 the treasury is" now reads "at 100 the treasury is paid nothing".

Suggestion — t.Run. Done, for all three tests; cases name themselves as pctg=37/proposalID=1337, which matters more now that there are 101 of them.

Warning 2 — require.ErrorContains couples to error wording. Declining. There are no sentinel errors to switch to: EncodeShastaExtraData builds its three errors inline with errors.New/fmt.Errorf. Introducing exported sentinels in input.go would be production churn for a test-only benefit, and ErrorContains is the established convention here — 49 uses across taiko-client's tests. The coupling is also what makes the test worth having: it pins that each invalid input produces its own rejection, which require.Error alone would not catch. A reworded message failing this test is a visible, one-line fix, not a silent regression.

---

Generated by Claude Code

[github-actions]

🐋 DeepSeek Code Review

🟡 Warnings

• push path filters are missing !**/*.md. Both workflows now trigger on packages/protocol/**, but only the pull_request block excludes markdown globally. A docs/README-only protocol change merged to main will now run the full client integration suite unnecessarily. Add - "!**/*.md" to both push blocks for consistency.

• The long protocol path-filter list is duplicated four times across the two workflows and across push/pull_request in each file. This will drift from protocol.yml over time, recreating the same class of silent CI gap this PR fixes. Consider a YAML anchor per workflow at minimum, or a shared/centrally generated path list if the repo has a mechanism for that.

• Error-message assertions in the new Go tests are brittle. TestEncodeShastaExtraDataRejectsInvalidProposalID pins exact substrings like "proposal ID too large". If the encoder changes to a wrapped or more descriptive error, these tests fail even if the validation behavior is correct. Prefer sentinel/exported errors or drop the message check to require.Error.

🔵 Suggestions

• Use require.NoError(t, err) instead of require.Nil(t, err) for the error returns in the new encoding tests; it gives better failure output and is idiomatic for errors.

• If contract_layout_* can ever be directories rather than flat files, the exclusion !packages/protocol/contract_layout_* will not match files inside them. Use !packages/protocol/contract_layout_*/** to be safe.

• The basefeeSharingPctgRange() helper is called multiple times and allocates a 101-element slice each time. It’s negligible in tests, but a package-level precomputed slice would avoid the repeated allocation.

🟢 What Looks Good

• Correctly removes the main-pinned second checkout and makes PROTOCOL_DIR point at the PR’s own packages/protocol.
• New encoding tests cover the full accepted basefeeSharingPctg range, the max uint48 proposal ID, and invalid proposal ID cases.
• The workflow comment documents why broad protocol paths are used rather than a narrow hand-picked contract path list.
• The new test verifies field independence between basefeeSharingPctg and proposal ID, which directly targets the overlapping-byte risk in extraData.

---

Automatically triggered on PR update • model: deepseek-v4-pro

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 40.80%. Comparing base (53776d6) to head (f5f3e40).
⚠️ Report is 7 commits behind head on main.

Additional details and impacted files

---

Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 85ab553...f5f3e40. Read the comment docs.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.