Skip to content

fix: reject log retrieval requests for not-in-context contracts#22047

Merged
mverzilli merged 5 commits into
merge-train/fairiesfrom
martin/validate-individual-retrieve-log-requests
Mar 27, 2026
Merged

fix: reject log retrieval requests for not-in-context contracts#22047
mverzilli merged 5 commits into
merge-train/fairiesfrom
martin/validate-individual-retrieve-log-requests

Conversation

@mverzilli

@mverzilli mverzilli commented Mar 26, 2026

Copy link
Copy Markdown
Contributor

Closes F-476

@mverzilli mverzilli requested review from benesjan and nchamo March 26, 2026 12:43
@mverzilli mverzilli changed the base branch from next to merge-train/fairies March 26, 2026 13:57
@mverzilli mverzilli added the claudebox Owned by claudebox. it can push to this PR. label Mar 26, 2026
@mverzilli

mverzilli commented Mar 26, 2026

Copy link
Copy Markdown
Contributor Author

/claudebox fix conflicts

@AztecBot

AztecBot commented Mar 26, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

⏳ Run #1 — Session completed (3m)
Live status

Resolved merge conflicts in #22047 — all 3 files had the same fetchLogsByTag → bulkRetrieveLogs rename conflict, kept the PR's version. Details: https://gist.github.com/AztecBot/bb83999360f6ed9c380ee649064efcb8

Base automatically changed from merge-train/fairies to next March 26, 2026 17:56
benesjan
benesjan reviewed Mar 27, 2026
View reviewed changes

@benesjan benesjan 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.

Given that we already have the contract address in the bulkRetrieveLogs then why do we even have it in the LogRetrievalRequest?

Shouldn't we just drop it from there instead of doing this check?

I think the address might have gotten in the LogRetrievalRequest at a time when we were considering whether we will want a contract B to process contract A's logs but that I have been abandoned.

I am not sure if it's abandoned for good though but if it is then I would just drop the contract address from the request.

Comment thread yarn-project/pxe/src/logs/log_service.ts Outdated
@mverzilli

mverzilli commented Mar 27, 2026

Copy link
Copy Markdown
Contributor Author

Given that we already have the contract address in the bulkRetrieveLogs then why do we even have it in the LogRetrievalRequest?

Shouldn't we just drop it from there instead of doing this check?

I think the address might have gotten in the LogRetrievalRequest at a time when we were considering whether we will want a contract B to process contract A's logs but that I have been abandoned.

I am not sure if it's abandoned for good though but if it is then I would just drop the contract address from the request.

I agree with this and we also discussed it with @nchamo, but changing the structure of LogRetrievalRequest would constitute a breaking change of oracle interface at this point. This PR just adds an additional validation working with the current interfaces

@mverzilli @benesjan
Update yarn-project/pxe/src/logs/log_service.ts
a94eb14 
Co-authored-by: Jan Beneš <janbenes1234@gmail.com>
@mverzilli mverzilli changed the base branch from next to merge-train/fairies March 27, 2026 09:59
@mverzilli

mverzilli commented Mar 27, 2026

Copy link
Copy Markdown
Contributor Author

/claudebox keep the fetchLogsByTag naming

@AztecBot

AztecBot commented Mar 27, 2026

Copy link
Copy Markdown
Collaborator

ClaudeBox: _keep the fetchLogsByTag naming _ ... workflow run

@mverzilli mverzilli enabled auto-merge (squash) March 27, 2026 10:58
@mverzilli mverzilli merged commit bc7570d into merge-train/fairies Mar 27, 2026
11 checks passed
@mverzilli mverzilli deleted the martin/validate-individual-retrieve-log-requests branch March 27, 2026 11:34
AztecBot added a commit that referenced this pull request Mar 27, 2026
@mverzilli @AztecBot @benesjan
fix: reject log retrieval requests for not-in-context contracts (#22047)
7967002 
Closes F-476

---------

Co-authored-by: AztecBot <tech@aztec-labs.com>
Co-authored-by: Jan Beneš <janbenes1234@gmail.com>
@AztecBot

AztecBot commented Mar 27, 2026

Copy link
Copy Markdown
Collaborator

✅ Successfully backported to backport-to-v4-next-staging #22091.

This was referenced Mar 27, 2026
Open
Merged
AztecBot added a commit that referenced this pull request Mar 27, 2026
@AztecBot
fix(pxe): update log_service tests to use new fetchLogsByTag signature
309ad5c 
Missing contractAddress argument in 3 test calls after merging next into
merge-train/fairies. The fairies PR #22047 added contractAddress as first
param but some tests from next still used the old signature.
This was referenced Mar 27, 2026
Open
Open
AztecBot added a commit that referenced this pull request Mar 27, 2026
@AztecBot
fix: reject log retrieval requests for not-in-context contracts (#22047)
5889f83
@AztecBot AztecBot mentioned this pull request Mar 27, 2026
Closed
github-merge-queue Bot pushed a commit that referenced this pull request Mar 27, 2026
@mverzilli
feat: merge-train/fairies (#22094)
e9e4396 
BEGIN_COMMIT_OVERRIDE
fix: reject log retrieval requests for not-in-context contracts (#22047)
fix: add missing contractAddress param in log_service tests (#22098)
END_COMMIT_OVERRIDE
@AztecBot AztecBot mentioned this pull request Mar 27, 2026
Merged
AztecBot added a commit that referenced this pull request Mar 28, 2026
@AztecBot
chore: Accumulated backports to v4-next (#22091)
9f02136 
BEGIN_COMMIT_OVERRIDE
fix(stdlib): correct NoteDao size (#22068)
fix: reject log retrieval requests for not-in-context contracts (#22047)
refactor: remove aztec dependency from aztec_sublib (#22033)
END_COMMIT_OVERRIDE
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@benesjan benesjan benesjan approved these changes

@nchamo nchamo nchamo approved these changes

Assignees

No one assigned

Labels

backport-to-v4-next claudebox Owned by claudebox. it can push to this PR.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@mverzilli @AztecBot @benesjan @nchamo