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.

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

Rlp decode #13

Draft
wants to merge 6 commits into
base: ref-scroll-stable
Choose a base branch
from
Draft

Rlp decode #13

wants to merge 6 commits into from

Conversation

smtmfft
Copy link
Owner

@smtmfft smtmfft commented Apr 4, 2023

The basic idea is to reuse as more as possible scroll's rlp_circuit which has full of encoding logic as its input is a tx_list and output is a rlp(tx_list).
Here in my design, the input will be raw rlp bytes, and output is a tx_list. And to let invalid rlp bytes come in, the circuit insures that either prover could give a correct bytes, or the whole tx_list is empty.

Changes:

  • remove TxSign rlp table as we don't need it.
  • remove extra tx rlp info rows.
  • add few columns for the list prefix
  • add logic for invalid rlp bytes input

TODOs:

  • add instance column if it's part of PI.
  • refactor by merging common logics.
  • make q_usable/q_end advice if necessary.
  • support place_holder bytes.
  • add tx_list rlp info rows if necessary.

To focus on changing constraints, I made this PR against the scroll's rather than PSE's.

@Brechtpd
Copy link

Brechtpd commented Apr 5, 2023

I'm still reviewing, but my initial thoughts are that quite a few changes are necessary, and the scroll circuit isn't exactly written in a very extensible way. So I'm not so sure anymore if basing our code on scroll's circuit is the best idea because it's already quite a complex circuit, and it's going to get even more complex. And if we have to extend the circuit with more multiple transaction types I think it'll be a pain to do so. In it's current state it's also very hard to know if all constraints are there because of the "flat list of constraints" style of scroll's circuit which I'm really not a fan of anymore.

I feel like this should also be a circuit should look closer to normal code. Maybe a good way start with that is to write the simplest code we can (in plain rust so easy to experiment and then try to get our circuit code to look as close as possible to that normal rust version) that only does the RLP data checking because that's the only new part we really need (we can still depend on the scroll version to do the rest).

I'll think about it some more tomorrow but let me know if you already have some feedback on this.

@smtmfft
Copy link
Owner Author

smtmfft commented Apr 5, 2023

Totally agree. Actually I was struggling at writing constraints based on these existed logics. And think better not to do so as long as that circuit is not meant to be extendable. I think we could have a more clear circuit which serves our purpose better if we draw it from very beginning with the experiences from this POC.

In it's current state it's also very hard to know if all constraints are there because of the "flat list of constraints" style of scroll's circuit which I'm really not a fan of anymore.

Yes, it's hard to tell if all corners are covered by this plain list, I think native like code could be helpful at least in syntax way. worth to try. Having a vague feeling that circuit DSL might eventually outperform such low level plain api list.

@Brechtpd
Copy link

Brechtpd commented Apr 5, 2023

Alright let's write a new circuit. Starting from the plain rust code I think is good because it's a nice target and easy to test for any missing checks, that way we know for sure we know all the checks we need to have.

Then for the circuit implementation I kind of think we can use the circuit tools I made for the MPT circuit, though likely not exactly the best for a circuit like this with what it can currently do but ehhh probably good enough, we can see how things can be improved for these types as circuits. Something like what axiom does could also be interesting for these types of circuits that can be very narrow, but writing circuits in two completely different ways is also not great so better to improve something more universally usable which hopefully can be the circuit tools.

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.

2 participants