Skip to content

Re-enable commented-out integration test matrix entries in tests.yml #1945 - #1953

Draft
adecaro wants to merge 11 commits into
mainfrom
1942-policy-based-identity-tests-are-not-enabled
Draft

Re-enable commented-out integration test matrix entries in tests.yml #1945#1953
adecaro wants to merge 11 commits into
mainfrom
1942-policy-based-identity-tests-are-not-enabled

Conversation

@adecaro

@adecaro adecaro commented Jul 19, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Summary

Re-enables the commented-out itest matrix entries in .github/workflows/tests.yml (dlog-fabric-t14, fabricx-dlog-t2, fabricx-dlog-t2.1, fabricx-dlog-t7, fabricx-dlog-t9, fabricx-dlog-t12, fabricx-dlog-t14) and fixes the underlying bugs that were causing each of them to fail, rather than re-disabling them:

  • fabricx-dlog-t2 / fabricx-dlog-t2.1 — the finality view's wait context was rooted in the P2P stream that delivered the triggering message. That stream can be closed for reasons unrelated to the wait (refcount reaching zero, an unrelated read error, session teardown), which canceled the context and surfaced as a false "finality timeout" well before the real deadline. Detached the wait context with context.WithoutCancel.
  • fabricx-dlog-t7 — the wait for issuer/auditor public-params propagation used a 30s timeout with 15s polling (1-2 attempts total), too tight for the update to reliably land. Widened to 60s timeout / 5s polling.
  • fabricx-dlog-t9 — MaliciousTransferView deliberately reuses the same computed tx_id across two separate CollectEndorsementsView calls, each independently inserting a validation record on the endorser side. The second insert violated the tx_id primary key. Added ON CONFLICT DO NOTHING so a duplicate validation record for an already-seen tx_id is a no-op, plus a regression test.
  • fabricx-dlog-t12 — CheckCoOwnedBalanceForTMSID did a single synchronous balance check, unlike its sibling CheckBalanceForTMSID which polls with gomega.Eventually. After a multisig lock finalizes, a co-owner's own view of the new co-ownership can lag slightly, causing a flaky one-shot check. Wrapped it in the same eventual-polling pattern.
  • dlog-fabric-t14 / fabricx-dlog-t14 — extractIssueOutputs/extractTransferOutputs treated every recipient of a multi-recipient owner identity (e.g. multisig/policy) as an independent output owner and fanned out one Output per co-owner, each showing the full quantity. For a composite identity, the recipients are co-owners of a single output. Now emits one Output for the composite owner after validating each co-owner's recipient metadata.

Fixes #1945

Test plan

  • make checks
  • make lint-auto-fix
  • go test ./token/...
  • Targeted fix validated with new/updated tests: EDuplicateValidationRecord (sqlite + memory drivers), TestAWAddValidationRecordEndorser (sqlite + postgres)
  • Re-enabled CI matrix entries verified against the previously failing PR Re-enable commented-out integration test matrix entries in tests.yml #1945 #1953 CI logs (ci/scripts/get-pr-failed-logs.sh), diagnosed one by one

@adecaro adecaro added this to the Q3/26 milestone Jul 19, 2026
@adecaro adecaro self-assigned this Jul 19, 2026
@adecaro
adecaro marked this pull request as draft July 19, 2026 15:13
@adecaro adecaro linked an issue Jul 19, 2026 that may be closed by this pull request
Re-enable commented-out integration test matrix entries in tests.yml #1945
Open
@adecaro adecaro changed the title policy-based identity tests are not enabled #1942 Re-enable commented-out integration test matrix entries in tests.yml #1945 Jul 19, 2026
@adecaro
adecaro force-pushed the 1942-policy-based-identity-tests-are-not-enabled branch from ae4d48b to f4c10fa Compare July 26, 2026 18:35
@adecaro
adecaro marked this pull request as ready for review July 27, 2026 11:40
@adecaro
adecaro marked this pull request as draft July 27, 2026 12:22
AkramBitar
AkramBitar approved these changes Jul 27, 2026
View reviewed changes

@AkramBitar AkramBitar left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@adecaro
adecaro force-pushed the 1942-policy-based-identity-tests-are-not-enabled branch from 823dcf0 to 6132b54 Compare July 28, 2026 07:00
adecaro added a commit that referenced this pull request Jul 28, 2026
@adecaro
fix(ttx): restrict distribution list to selected policy signers
d32ce37 
prepareDistributionList unwrapped a policy identity's components
unconditionally, while requestSignatures already filtered them through
policyCollectIDs. When an OR-policy spend restricts signing to a
subset of co-owners via WithPolicySigners, the excluded co-owner still
received the assembled transaction even though it never opened a
session expecting one, causing RespondRequestRecipientIdentityView to
panic with "expected recipient_req, got transaction". This was the
root cause of the dlog-fabric-t14/fabricx-dlog-t14 integration test
failures confirmed on PR #1953 CI (6/6 spec variants across both
drivers and all transports).

Extract the unwrap loop into unwrapDistributionIDs and filter policy
components through policyCollectIDs, mirroring requestSignatures.

Signed-off-by: Angelo De Caro <adc@zurich.ibm.com>
adecaro and others added 11 commits July 29, 2026 08:34
@adecaro
enable all tests
0bef878 
Signed-off-by: Angelo De Caro <adc@zurich.ibm.com>
@adecaro
fix(ttx): detach finality wait from responder stream context
7a066a2 
The finality view's context was rooted in the P2P stream that delivered
the message that created it. That stream can be closed for reasons
unrelated to the finality wait (refcount hitting zero, an unrelated
read error, session teardown), which cancels the context and surfaces
as a false "finality timeout" well before the real deadline. Detach
with context.WithoutCancel and rely solely on the explicit timeout.

Fixes fabricx-dlog-t2 and fabricx-dlog-t2.1.

Signed-off-by: Angelo De Caro <adc@zurich.ibm.com>
@adecaro
fix(integration): loosen public params propagation wait
f8e5067 
The wait for issuer/auditor public-params propagation used a 30s
timeout with 15s polling (i.e. only 1-2 attempts), too tight for the
update to land reliably. Widen to 60s timeout with 5s polling.

Fixes fabricx-dlog-t7.

Signed-off-by: Angelo De Caro <adc@zurich.ibm.com>
@adecaro
fix(storage): make validation record insert idempotent
64e800a 
MaliciousTransferView reuses the same computed tx_id across two
separate CollectEndorsementsView calls, each triggering an independent
AddValidationRecord insert on the endorser. The second insert violated
the tx_id PRIMARY KEY constraint. Add ON CONFLICT DO NOTHING so a
duplicate validation record for an already-seen tx_id is a no-op
instead of an error, and add a regression test covering it.

Fixes fabricx-dlog-t9.

Signed-off-by: Angelo De Caro <adc@zurich.ibm.com>
@adecaro
fix(integration): retry co-owned balance check with eventual polling
52203d8 
CheckCoOwnedBalanceForTMSID performed a single synchronous check,
unlike its sibling CheckBalanceForTMSID which polls with
gomega.Eventually. After a multisig lock finalizes, co-owners'
own view of their new co-ownership can lag slightly behind,
causing a flaky one-shot check. Wrap it in the same eventual
polling pattern.

Fixes fabricx-dlog-t12.

Signed-off-by: Angelo De Caro <adc@zurich.ibm.com>
@adecaro
fix(token): emit a single output for composite-owner recipients
3368161 
extractIssueOutputs/extractTransferOutputs treated every recipient in
a multi-recipient owner identity as an independent output owner and
fanned out one Output per co-owner. For a composite identity (multisig/
policy) the recipients are co-owners of a single output, not separate
owners each entitled to the full quantity. Emit one Output for the
composite owner instead, after validating each co-owner's recipient
metadata.

Fixes dlog-fabric-t14 and fabricx-dlog-t14.

Signed-off-by: Angelo De Caro <adc@zurich.ibm.com>
@adecaro @claude
fix(fungible): recognize composite-owner outputs in multisig accept view
1855aa9 
extractIssueOutputs/extractTransferOutputs now emit a single output owned
by the composite multisig/policy identity when a token has multiple
recipients, but AcceptCashView still checked outputs.ByRecipient(id)
against each responder's individual identity. The strict equality check
never matched the composite owner, so the existence assertion panicked
on bob's/charlie's node and the per-type balance check was silently
skipped, breaking dlog-fabric-t12 (EndToEnd Multisig) across all three
P2P transports.

Add OutputStream.ByRecipientOrMember, mirroring the membership check
already used in AssertTokens, and use it in AcceptCashView so a
responder recognizes outputs owned by a composite identity she is a
member of.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Signed-off-by: Angelo De Caro <adc@zurich.ibm.com>
@adecaro @claude
remove plan.md
36053fc 
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Signed-off-by: Angelo De Caro <adc@zurich.ibm.com>
@adecaro
fix(ttx): match policy signers by IsMe fallback, poll policy balance …
f5f67d2 
…checks

policyCollectIDs only matched WithPolicySigners entries by exact identity
bytes, but dlog/zkatdlog owner wallets record a pseudonymous component
identity in the PolicyIdentity that differs from the raw network identity
callers pass to WithPolicySigners. This left collectIDs empty for OR-policy
spends, so the policy signature verified as unsatisfied even though the
intended signer had signed. Fall back to SigService().IsMe when the exact
match fails, mirroring the membership check already used for composite
owners elsewhere.

CheckPolicyOwnedBalanceForTMSID read the balance once with no retry, unlike
its sibling Eventually-wrapped balance checks in the same file, making it
racy against eventual consistency after a policy spend commits.

Signed-off-by: Angelo De Caro <adc@zurich.ibm.com>
@adecaro
fix(ttx): restrict distribution list to selected policy signers
4593f94 
prepareDistributionList unwrapped a policy identity's components
unconditionally, while requestSignatures already filtered them through
policyCollectIDs. When an OR-policy spend restricts signing to a
subset of co-owners via WithPolicySigners, the excluded co-owner still
received the assembled transaction even though it never opened a
session expecting one, causing RespondRequestRecipientIdentityView to
panic with "expected recipient_req, got transaction". This was the
root cause of the dlog-fabric-t14/fabricx-dlog-t14 integration test
failures confirmed on PR #1953 CI (6/6 spec variants across both
drivers and all transports).

Extract the unwrap loop into unwrapDistributionIDs and filter policy
components through policyCollectIDs, mirroring requestSignatures.

Signed-off-by: Angelo De Caro <adc@zurich.ibm.com>
@adecaro
fix(fungible): wait for FSC endorsers to catch up in TestPublicParams…
a1f5301 
…Update

TestPublicParamsUpdate updates public parameters and waited for the
issuer/auditor nodes to observe them, but never waited for FSC
endorser nodes (present under FSCBasedEndorsement, e.g. fabricx). The
endorser validates auditor signatures against its own locally cached
public params, so a subsequent issue with the new auditor/issuer could
race ahead of the endorser's update and fail with "auditor [...] is
not in auditors". Mirrors the existing wait pattern already used in
TestAll.

Signed-off-by: Angelo De Caro <adc@zurich.ibm.com>
@adecaro
adecaro force-pushed the 1942-policy-based-identity-tests-are-not-enabled branch from 88b7be0 to a1f5301 Compare July 29, 2026 06:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@AkramBitar AkramBitar AkramBitar approved these changes
@KElkhiyaoui KElkhiyaoui Awaiting requested review from KElkhiyaoui
@alexandrosfilios alexandrosfilios Awaiting requested review from alexandrosfilios

Assignees

@adecaro adecaro

Labels

testing

Projects

None yet

Milestone

Q3/26

Development

Successfully merging this pull request may close these issues.

Re-enable commented-out integration test matrix entries in tests.yml

2 participants

@adecaro @AkramBitar