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

cryptex milestone 2 #985

Merged
merged 1 commit into from
Sep 13, 2023
Merged

cryptex milestone 2 #985

merged 1 commit into from
Sep 13, 2023

Conversation

driemworks
Copy link
Contributor

Milestone Delivery Checklist

  • The milestone-delivery-template.md has been copied and updated.
  • The invoice form 📝 has been filled out for this milestone.
  • This pull request is being made by the same account as the accepted application.
  • I have disclosed any and all sources of reused code in the submitted repositories and have done my due diligence to meet its license requirements.
  • In case of acceptance, the payment will be transferred to the BTC/ETH/fiat account provided in the application.
  • The delivery is according to the Guidelines for Milestone Deliverables.

Link to the application pull request: w3f/Grants-Program#1660

@dsm-w3f dsm-w3f self-assigned this Aug 29, 2023
@dsm-w3f
Copy link
Contributor

dsm-w3f commented Aug 29, 2023

@driemworks thank you for the milestone delivery. Please see the evaluation document and provide proper answers and fixes. Let me know when I can continue this evaluation.

@driemworks
Copy link
Contributor Author

driemworks commented Aug 30, 2023

@dsm-w3f thanks for the quick feedback! I've resolved all the issues in the evaluation document. To summarize:

  • Docker: Only substrate is dockerized. The SDK can be used as either a typescript lib (wasm build) or module (rust), so there's nothing to run in a container with that alone. Similarly, the typescript wrapper (etf.js) has no docker image either. You can find both packages in npm here. I can add a dockerfile for the etf.js/examples if you'd like.

  • Substrate Issues: resolved

  • ETF-SDK:

    1. cleaned up test imports/params
    2. No need to deploy anything here. If you'd like to locally verify that the latest build works rather than relying on the package in npm you can build from the api directory with wasm-pack build --target web, then you just need to change the imports in etf.js here.
  • etf.js

    1. fixed that test, a rounding issue
    2. I added a temporary fix to the example to get rid of the source map warning here. We'll properly address that when we build out the 'real' use case in the next milestone.

@dsm-w3f
Copy link
Contributor

dsm-w3f commented Aug 30, 2023

@driemworks thank you for the answers and fixes. I tried again and I still have some doubts and problems. Please see the evaluation document. Let me know when I can continue the evaluation.

@driemworks
Copy link
Contributor Author

  • regarding benchmarks + tests in substrate:
    • are you able to run cargo test --package pallet-etf --features runtime-benchmarks?
    • I'm not sure of the errors you're seeing, but I believe they're things that already exist within substrate. The pallet-timestamp and election-provider errors that are logged seem intentional (tests with error logs have named like enters_emergency_phase_after_forcing_before_elect and on_new_session_doesnt_start_new_set_if_schedule_change_failed ). I'll look into those to verify. The errors logs in correct_beefy_payload is unexpected, but we would not have impacted that and our fork is somewhat outdated at this point, so perhaps they'll be resolved when we sync with the latest (soon).
  • regarding etf.js:
    • you can also generate the dist folder by running tsc from the root dir
    • 😢 apologies about that test, it is resolved 100% now.
    • technically the example does run everything locally, because it's running a smoldot light client in you browser. You can connect with a full node like this: run a full node locally, then change the example here to pass the host and port and set the init flag to false. e.g.
      const distanceBasedSlotScheduler = new DistanceBasedSlotScheduler();
      let api = new Etf(distanceBasedSlotScheduler, "0.0.0.0", 9944);
      await api.init(false);
    It's possible but I don't believe it's practical to run a bootnode locally and test with the smoldot light client because smoldot has some TLS requirements (when we attempted we received a protocol not supported over an insecure connection) but I can share logs or access to the hosted bootnode (in AWS) if desired.

@dsm-w3f
Copy link
Contributor

dsm-w3f commented Sep 1, 2023

@driemworks thank you for the improvements and fixes. I was able to test more parts locally but not all the system. Please see the evaluation document for details. Let me know when I can continue this evaluation.

@driemworks
Copy link
Contributor Author

The smoldot fork is working, but I don't know how to add and run with local chain 127.0.0.1:9944

There are basically two ways to run everything:

  1. run smoldot in the browser with a hosted bootnode
  2. locally run a full node optionally connected to a hosted bootnode.
    and of course if you're running a hosted bootnode, you can add both full and light client peers

The first way is what exists in the etf.js/examples app, which syncs with the etf0 bootnode. For the second approach, where you want to use a locally running node instead of a smoldot light client, you'd have to make the change like in my last comment (replace lines 25 and 26 here ). You won't be able to connect the smoldot client with a locally running full node I believe (correct me if wrong, but smoldot doesn't support insecure connections so it's possible but you'd need to set up a secure connection between local full node and smoldot). I attempted previously by updating the chainspec to point to a locally running bootnode but receive protocol not supported and the lc can't make a connection.

You could run the full node a couple of ways, either as its own validator on its own chain, starting at block zero, or else get the chainspec here to sync with the etf0 bootnode. If you connect with the light client right now it might take a minute or two to sync.

If you're available for a call we can probably clear everything up.

@dsm-w3f
Copy link
Contributor

dsm-w3f commented Sep 4, 2023

@driemworks thank you for the answer. I tried to test it again locally using Smoldot and didn't work, but as worked with the full node, I think maybe there is some detail that I'm missing. Please see the evaluation document for details. I also noticed that the evaluation branch was not updated with some changes that I need to make to run the code, for example, the merge conflicts in the code. I expect that the user could just follow the instructions in the Readme files and run the application. Please make sure that the documentation and code incorporate the changes needed to allow that. There is no need for a meeting, the documentation should be enough to run the system.

Also pinging @burdges, to see if he could take a look at the cryptography part of this milestone delivery since he was actively involved in the discussions of the grant application.

@driemworks
Copy link
Contributor Author

I started the smoldot using npm install; npm start in ./wasm-node/javascript.

You don't need to do this actually, etf.js starts the light client in the browser. You should be able to just pull the latest main branch and run the example following the readme. There was a problem with the event emitter's visibility that is resolved that was preventing the latest slot from being dispatched to the react app.

I removed the /dist folder from git, since it should be built from the root directory anyway (using tsc).

@dsm-w3f
Copy link
Contributor

dsm-w3f commented Sep 5, 2023

@driemworks thank you for the answer. I tried and the main branch is more stable than the other one passed before. For now, I will wait for feedback of the research team regarding the cryptography part. This could take one or two weeks to complete.

@driemworks
Copy link
Contributor Author

@dsm-w3f sounds good. I'm available via matrix as well (@driemworks:matrix.io) to provide any further context regarding the cryptography aspects of the codebase (which somewhat fall out of the scope of the testing docs we provided).

@burdges
Copy link

burdges commented Sep 6, 2023

It sounds like you've a bigger vec than you'd like? https://github.com/ideal-lab5/etf-sdk/blob/main/crypto/src/ibe/fullident.rs#L20

I've now forgotten exactly what you wanted to do, but assuming you wanted a threshold scheme, then you can have the user do Lagrange interpolation between the g_ids, no?

It's likely all fine, but please do feel free to ask about optimizing the size.

@driemworks
Copy link
Contributor Author

That was related to the logic here, but I think it's ok. The function uses a threshold scheme to generate an AES key, and then encrypts the shares using IBE, and later on the decrypt function decrypts the shares (assuming slot secrets are available) and uses lagrange interpolation here to recover the AES key. My comment was more about a concern that the size of the resulting ciphertext could get really big if they want to choose a larger number of slots to encrypt to, but I think that's probably more of an application issue than implementation problem (i.e. if you only have X storage available, only use Y slots). It would be ideal if somehow the IBE ciphertexts could be aggregated but that's probably out of scope for us (for now at least).

I think my biggest concern is our DLEQ impl. You had previously mentioned using the dleq_vrf crate to do it, but we implemented it with arkworks instead. The current setup uses onchain randomness to seed an RNG when creating the proof. Would the dleq_vrf approach be more efficient?

@burdges
Copy link

burdges commented Sep 6, 2023

It's likely fine..

You do need an "IBE encryption" per future block the user selects, but you could think if them not so much as encryptions but as key exchanges. Set kex(i) := h2(pairing(esk * h2c(seed[i]),mpk[i]). You then define a q: Vec<F> and an interpolation polynomial p(x) so that so that p(i) = kex(i) for 1<=i <= k and p(i) = kex(i) + q[i] for i>k.

In your ciphertext, you need the q.len = n-k field elements in q, what blocks the is denote, which then determines seed[i],mpk[i], and you need u[i] = esk * h2c(seed[i]) for the number of seed that differ, likely all n unless you do something strange.

I guess q and u are the two lists you have now, but then you likely want a MAC list too, so whoever decrypts knows which decryption keys succeed vs fail. You'd encrypt the final message with p(0) and provide another MAC so people know if the original user lied. So n+1 MACs in total.

I didn't notice these MACs but maybe you've them in your encryption somehow.

@driemworks
Copy link
Contributor Author

Thanks! That seems much more optimal. Instead of performing IBE encryption ala FullIdent, we just perform this key exchange and don't need to store/transport the larger IBE ciphertexts. There is no MAC currently, it just ignores invalid keys right now. We'll plan to make both of these alterations in the next milestone.

@burdges
Copy link

burdges commented Sep 6, 2023

It's likely FullIdent contains a MAC of some sort, not sure.

You're almost still performing one "encryption" per upcoming block, well the MACs can be thought of as an AEAD of the empty message, but MACs only require 16 bytes each. It's the u[i] parts of the cyphertexts which take up 48 bytes each, so you need like 64 * n + f * (n-k) bytes where f is the symmetric key field size.

It's likely fine if f is only 128 bits, because what would collision attacks achieve? That's not a proof though, just a guess.

@driemworks
Copy link
Contributor Author

It's likely FullIdent contains a MAC of some sort, not sure.

The decryption function uses the first param of the ciphertext to authenticate (checks if U == rP, in G2) so sort of a MAC, but it's 96 bytes per ciphertext and each ciphertext ends up being 160 bytes (using sha256), so it's larger than the above for n > 2 as long as k is large enough?

@driemworks
Copy link
Contributor Author

@dsm-w3f when do you think we can proceed? We intend to implement the optimizations discussed as part of the next milestone, since it will require further research, implementation and testing, and the current implementation seems to be working correctly (though not optimally).

@dsm-w3f
Copy link
Contributor

dsm-w3f commented Sep 13, 2023

@driemworks thank you for the answers and @burdges thanks for the review. I agree that these optimizations can be handled later. The milestone is approved. I'll forward your invoice internally and the payment should take place within two weeks. Great job!

@dsm-w3f dsm-w3f merged commit 5d31ab1 into w3f:master Sep 13, 2023
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants