Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat(protocol): fix signal service multi-hop proof verification bugs #15680

Merged
merged 98 commits into from
Feb 10, 2024
Merged

feat(protocol): fix signal service multi-hop proof verification bugs #15680

merged 98 commits into from
Feb 10, 2024

Conversation

dantaik
Copy link
Contributor

@dantaik dantaik commented Feb 8, 2024 •
edited
Loading

There are two bugs:

  1. We should not allow users to specify the crossChainSync address, it should always be the "taiko" address on the curernt chain.
  2. We should check that each hop relay -- the middleman contract that sends stateRoot as a signal to its local signal service. Previously this address is not verified correctly using the AuthorizableContract contract. Now we removed AuthorizableContract and introduce a HopRelayRegistry to verify hop relays explicitly. Note that if the signal service disabled multi-hop bridging, the HopRelayRegistry is not needed.

dantaik and others added 30 commits February 5, 2024 17:29
@dantaik
breaking change by removing signal service overhead
ab12a57
@dantaik
Merge branch 'alpha-6' into alpha-7
3744fc8
@dantaik
Merge branch 'alpha-6' into alpha-7
b5728d1
@dantaik
remove signalRoot step 1
a0f5efb
@dantaik
Update LibProving.sol
019ef54
@dantaik
more
45e8ceb
@dantaik
Update LibUtils.sol
9224460
@dantaik
Update LibVerifying.sol
758ab07
@dantaik
more
71396f4
@dantaik
Update TaikoL2.sol
5aa18f5
@dantaik
Update TaikoL2.sol
b050af5
@dantaik
Update TaikoL2.sol
f978d83
Make code compiling
7f48859
Add back helpers and uncomment lines
ed3d47c
@dantaik
fix anchor
d6def2a
@dantaik
Update TaikoL2.sol
27de76a
@dantaik
Update TaikoL2.t.sol
05dd063
@dantaik
minor
2e22f3e
@dantaik
rename
28134d6
@dantaik
rename
1e70e33
@dantaik
update multi-hop bridging diagrams
425be15
@dantaik
more error messages in signal service
7a2d7f2
@dantaik
Update SignalService.sol
0a809f9
@dantaik
Update SignalService.sol
27f51d3
@dantaik
Update SignalService.sol
2bd445b
@dantaik
Update SignalService.sol
8200313
@dantaik
comments
9ded143
@dantaik
Update LibVerifying.sol
bef6290
@dantaik
more
c6e72ff
@dantaik
Update TaikoL2.sol
528288d
@dantaik dantaik changed the title feat(protocol)!: fix signal service proof verification bugs feat(protocol)!: fix signal service multi-hop proof verification bugs Feb 9, 2024
@dantaik dantaik changed the title feat(protocol)!: fix signal service multi-hop proof verification bugs feat(protocol)!: fix signal service multi-hop proof verification bugs [wait for base PR to be merged first] Feb 9, 2024
dantaik and others added 5 commits February 9, 2024 20:27
@dantaik @Brechtpd
Update packages/protocol/contracts/L1/TaikoL1.sol
238f8a4 
Co-authored-by: Brecht Devos <Brechtp.Devos@gmail.com>
@dantaik @Brechtpd
Update packages/protocol/contracts/signal/SignalService.sol
93869d9 
Co-authored-by: Brecht Devos <Brechtp.Devos@gmail.com>
@dantaik @Brechtpd
Update packages/protocol/contracts/L2/TaikoL2.sol
57b73c5 
Co-authored-by: Brecht Devos <Brechtp.Devos@gmail.com>
@dantaik @Brechtpd
Update packages/protocol/contracts/signal/SignalService.sol
298bd20 
Co-authored-by: Brecht Devos <Brechtp.Devos@gmail.com>
@dantaik
Update PseZkVerifier.sol
c0b39c8
@dionysuzx dionysuzx changed the title feat(protocol)!: fix signal service multi-hop proof verification bugs [wait for base PR to be merged first] feat(protocol): fix signal service multi-hop proof verification bugs [wait for base PR to be merged first] Feb 9, 2024
@dionysuzx
Copy link
Collaborator

approving as merge master. i removed the ! as we wont worry about breaking change for now.

Brechtpd
Brechtpd approved these changes Feb 9, 2024
View reviewed changes
Copy link
Contributor

@Brechtpd Brechtpd left a comment

Choose a reason for hiding this comment

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

Approved because I think this correctly links the difference chains, but still with low certainty.

@dantaik
Copy link
Contributor Author

dantaik commented Feb 10, 2024

Approved because I think this correctly links the difference chains, but still with low certainty.

Lets disable multi-hop for now in the code.

Base automatically changed from remove-signal-root to main February 10, 2024 03:19
@dantaik dantaik changed the title feat(protocol): fix signal service multi-hop proof verification bugs [wait for base PR to be merged first] feat(protocol): fix signal service multi-hop proof verification bugs Feb 10, 2024
@dantaik dantaik enabled auto-merge (squash) February 10, 2024 03:35
@dantaik dantaik merged commit b46269c into main Feb 10, 2024
9 of 14 checks passed
@dantaik dantaik deleted the fix-multi-hop-trust-graph-issue branch February 10, 2024 03:35
@github-actions github-actions bot mentioned this pull request Feb 10, 2024
Merged
@taiko-kitty taiko-kitty mentioned this pull request Mar 1, 2024
Closed
@taiko-kitty taiko-kitty mentioned this pull request Mar 29, 2024
Closed
This was referenced Apr 5, 2024
Closed
Closed
@taiko-kitty taiko-kitty mentioned this pull request Apr 25, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dionysuzx dionysuzx dionysuzx approved these changes

@Brechtpd Brechtpd Brechtpd approved these changes

@adaki2004 adaki2004 Awaiting requested review from adaki2004

@davidtaikocha davidtaikocha Awaiting requested review from davidtaikocha

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@dantaik @dionysuzx @Brechtpd