Skip to content

fix(test-suite): fix delegated user decrypt assertion#2070

Open
tawadaa wants to merge 3 commits intomainfrom
aw/fix-delegation-decrypt
Open

fix(test-suite): fix delegated user decrypt assertion#2070
tawadaa wants to merge 3 commits intomainfrom
aw/fix-delegation-decrypt

Conversation

@tawadaa
Copy link
Copy Markdown
Contributor

@tawadaa tawadaa commented Mar 9, 2026

No description provided.

@tawadaa tawadaa requested a review from a team as a code owner March 9, 2026 18:08
@cla-bot cla-bot bot added the cla-signed label Mar 9, 2026
@mergify
Copy link
Copy Markdown

mergify bot commented Mar 9, 2026

🧪 CI Insights

Here's what we observed from your CI run for ab6d3c1.

🟢 All jobs passed!

But CI Insights is watching 👀

// Wait for 2 blocks to ensure delegation is propagated by the coprocessor.
const currentBlock = await ethers.provider.getBlockNumber();
await waitForBlock(currentBlock + 15);
await waitForBlock(currentBlock + 2);
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.

Do you know why this got decremented to 2 blocks? Is the simple ACL already working maybe?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

With 15 blocks we hit the timeout for the last test, as we wait for 15 blocks twice.
Not sure for simple ACL, as this tested for v0.11.

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.

I would appreciate if we could merge #2072 first, to avoid dealing with conflicts on a big long living feature branch 😇

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.

Simple ACL changes are not yet active on main.

The error matching would again change with simple ACL, as the check will be have a new error label for Host ACL related errors.

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.

I agree with @eudlins-zama that we should merge the feature branch sooner.

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.

Since simple ACL is on main, what is the next step on this PR?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@Eikix There were 2 main issues that this PR is resolving:

  1. Enough timeout for real blockchain networks
  2. Reject with the correct Error message

With simple ACL, we still need 1, but not sure if 2 was fixed

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.

So the PR is good to merge as is? or it needs rework?

Copy link
Copy Markdown
Contributor

@manoranjith manoranjith left a comment

Choose a reason for hiding this comment

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

LGTM.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants