Skip to content

feat(patterns): For containerHasSplit(copyArray, ...), scan from the start#3067

Merged
gibson042 merged 3 commits intomasterfrom
gibson-2026-01-containerhassplit-copyarray-direction
Feb 26, 2026
Merged

feat(patterns): For containerHasSplit(copyArray, ...), scan from the start#3067
gibson042 merged 3 commits intomasterfrom
gibson-2026-01-containerhassplit-copyarray-direction

Conversation

@gibson042
Copy link
Member

Ref #3065 (comment)

Description

Avoids returning result arrays with elements in reversed order.

Security Considerations

None known.

Scaling Considerations

n/a

Documentation Considerations

Ignored.

Testing Considerations

Updated tests from #3065.

Compatibility Considerations

This is a behavior change, but I don't think containerHasSplit even has use yet, nor do I expect reliance upon the counterintuitive behavior.

Upgrade Considerations

I think this fix falls below the level of inclusion in NEWS.md, but could be persuaded out of that position.

Copy link
Contributor

@erights erights left a comment

Choose a reason for hiding this comment

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

Be aware that a Set amount value necessarily lists its elements in descending order, just like the payload of a CopySet amount value. See

https://github.com/Agoric/agoric-sdk/blob/b5638874e8aa65b633fd348139fa4980f15670ff/packages/ERTP/src/mathHelpers/setMathHelpers.js#L26

 list = coerceToElements(list);

and

export const coerceToElements = elementsList => {

How does that affect the desirability or relevance of this change? (Not rhetorical. I genuinely don't know yet.)

@gibson042 gibson042 force-pushed the gh-3064-containerhassplit-hardened-results branch 3 times, most recently from 949cab5 to c488503 Compare February 25, 2026 06:33
Base automatically changed from gh-3064-containerhassplit-hardened-results to master February 25, 2026 06:43
@gibson042 gibson042 force-pushed the gibson-2026-01-containerhassplit-copyarray-direction branch from 32e6b7b to 941e84b Compare February 25, 2026 06:53
@changeset-bot
Copy link

changeset-bot bot commented Feb 25, 2026

🦋 Changeset detected

Latest commit: a58e436

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 4 packages
Name Type
@endo/patterns Minor
@endo/cli Patch
@endo/daemon Patch
@endo/exo Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@gibson042
Copy link
Member Author

Be aware that a Set amount value necessarily lists its elements in descending order, just like the payload of a CopySet amount value. See

https://github.com/Agoric/agoric-sdk/blob/b5638874e8aa65b633fd348139fa4980f15670ff/packages/ERTP/src/mathHelpers/setMathHelpers.js#L26

 list = coerceToElements(list);

and

export const coerceToElements = elementsList => {

How does that affect the desirability or relevance of this change? (Not rhetorical. I genuinely don't know yet.)

I'm actually not clear on what this is even asking, since containerHasSplit requires its input specimen to be a copyArray, copySet, or copyBag.

@gibson042 gibson042 force-pushed the gibson-2026-01-containerhassplit-copyarray-direction branch from 941e84b to d3fd205 Compare February 25, 2026 07:03
@erights
Copy link
Contributor

erights commented Feb 25, 2026

I'm actually not clear on what this is even asking, since containerHasSplit requires its input specimen to be a copyArray, copySet, or copyBag.

I no longer think it is relevant, so nevermind.

Comment on lines 1488 to 1489
outResults,
) &&
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
outResults,
-1,
) &&

Copy link
Contributor

Choose a reason for hiding this comment

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

or am I missing something?

Copy link
Member Author

@gibson042 gibson042 Feb 26, 2026

Choose a reason for hiding this comment

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

This is a call to pairsHasSplit, which is analogous to confirmElementsHasSplit but lacks a direction parameter because it's only applicable to CopyBag payloads and so always iterates by descending array index. There's coverage in packages/patterns/test/containerHasSplit.test.js , but I've just updated src/ comments as well.

Copy link
Contributor

@erights erights left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@gibson042 gibson042 force-pushed the gibson-2026-01-containerhassplit-copyarray-direction branch from e1c92b3 to aa68fed Compare February 26, 2026 21:40
@gibson042 gibson042 force-pushed the gibson-2026-01-containerhassplit-copyarray-direction branch from aa68fed to a58e436 Compare February 26, 2026 23:29
@gibson042 gibson042 enabled auto-merge February 26, 2026 23:30
@gibson042 gibson042 merged commit 8446e8e into master Feb 26, 2026
21 of 22 checks passed
@gibson042 gibson042 deleted the gibson-2026-01-containerhassplit-copyarray-direction branch February 26, 2026 23:36
@github-actions github-actions bot mentioned this pull request Feb 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants