Release: develop -> main - #4442
Merged
Merged
Conversation
* feat(custody): say outright whether the caller owns an account Clients had to infer ownership from the absence of the owner field, which is not a contract but a side effect: own accounts are loaded without the owner relation, foreign ones with it. Add the relation somewhere and the inference flips silently. It is not a cosmetic distinction. A client decides on it whether to offer transacting at all: orders carry no account, so an order placed while looking at someone else's Safe would be booked against the caller's own. Acting on another person's behalf does not exist here — initiatedBy is never set. isOwner states it. Creating an account yields true by definition; the update route answers it from the account, since a grantee passes the write guard too. * refactor(custody): ask the account who owns it The ownership comparison lived in seven places, one of them the controller — where the contributing guide says business logic does not belong. It is one method on the entity now, and the controller asks it. That also makes it testable, which the update route needed: it is the reason the flag exists, since a grantee passes the write guard without owning anything, and until now nothing exercised that. The entity spec covers it, including that an active grant does not make a grantee an owner. * refactor(custody): finish what the last commit claimed to have finished Three ownership comparisons were left behind: the multi-account shortcut in resolveOwnerAccountId, and the two grant guards. A textual search for the owner field misses the first, which is how the previous commit undercounted and then overclaimed. None of them behaved wrongly — they computed the same boolean — but three private copies of one predicate are three chances to drift. No raw comparison remains outside the entity.
* fix(custody): give the Safe balances a stable order The balances came back in whatever order the database chose — nothing orders that query. The same holdings could therefore appear in a different order on every request, with the list visibly reshuffling for no reason a customer could see. It also made screenshots of the Safe unreproducible. Largest position first, name as tiebreak. That is both stable and the order someone reading a portfolio expects. * fix(custody): rank a non-finite balance last instead of comparing it NaN makes every difference falsy, so a damaged value fell through to the name comparison — against every other entry, whatever its worth. The ordering lost its transitivity and the position depended on the input order again: exactly the unpredictability this sort was added to remove, only triggered by a broken figure instead of a missing ORDER BY. Reproduced before and after: the same four positions, fed forwards and backwards, now come out identical. It is ranked rather than thrown for the reason already recorded a few lines up in the caller — one damaged position must not take the customer's whole balance response down with it. Tests cover the negative case too. There is a negative balance row in production, so that is not hypothetical. * test(custody): make the ordering test actually catch the bug it names Its point is that the same holdings come back in the same order however they went in — but with three values that held for the broken comparator too, so the assertion would have watched the defect return without a word. Only the position checks were doing any work. Four values with a negative among them separate the two: the old comparator yields different orders forwards and backwards, the new one does not. Verified by putting the old line back and watching both tests go red. Also covers a real Infinity, which the ranking treats like NaN, and says in the comment why: not because infinities break the comparison, but because nothing legitimate produces one here — the same reading of corrupted data the interest calculation already applies.
TaprootFreak
approved these changes
Jul 29, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Automatic Release PR
This PR was automatically created after changes were pushed to develop.
Commits: 1 new commit(s)
Checklist