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

Clarify RAM footprint on MCUs #122

Closed
vhnatyk opened this issue Jun 19, 2019 · 11 comments
Closed

Clarify RAM footprint on MCUs #122

vhnatyk opened this issue Jun 19, 2019 · 11 comments

Comments

@vhnatyk
Copy link

vhnatyk commented Jun 19, 2019

Hi @real-or-random As you suggested I created a separate issue here in rust-secp256k1.

I'm on very very tiny Linux from Poky/Yocto project and have full std but only with 256KB of RAM available for my app. If I set some very small numbers to define size of precompiled tables like

ctx->prec = (secp256k1_ge_storage (*)[ECMULT_GEN_PREC_N][ECMULT_GEN_PREC_G])manual_alloc(prealloc, prealloc_size, base, prealloc_size);

so it effectively becomes

ctx->prec = (secp256k1_ge_storage (*)[16][16])manual_alloc(prealloc, 256, base, 256);

where prealloc_size is 256 due to SECP256K1_ECMULT_GEN_CONTEXT_PREALLOCATED_SIZE resolves to 256 in

 size_t const C= SECP256K1_ECMULT_GEN_CONTEXT_PREALLOCATED_SIZE;

Without those black magik tricks even with ECMULT_WINDOW_SIZE 4 - prealloc_size at some step is 65536 and causes SEGFAULT for me. With those small numbers, I'm able to run my code to the point where pure rust libsecp256k1 runs. But with those small numbers all major rust-secp256k1 sign/verify tests fail

though all tests in my repo pass. As I understand to avoid black magik I have to switch to static prec defining USE_ECMULT_STATIC_PRECOMPUTATION and generating ecmult_static_context.h with the smallest possible table size. Or since I neither use signing nor verify - just scalar and point math maybe I should disable pre-comp at all if I can? I saw SECP256K1_CONTEXT_NONE and in your PR Changes necessary for usage on Trezor secp256k1-zkp#53 and there is no method for context none in rust-secp256k1 lib.rs - just All, SignOnly, VerifyOnly.

Thanx)

@elichai
Copy link
Member

elichai commented Jun 19, 2019

does SECP256K1_CONTEXT_NONE works for you? or have you haven't tried?
If it does work I can add a PR for it

@vhnatyk
Copy link
Author

vhnatyk commented Jun 19, 2019

sorry I haven't tried yet, riding on black magik for now 🥂 - will try tomorrow asap

@elichai
Copy link
Member

elichai commented Jun 19, 2019

@vhnatyk Just edit L25 and L27 to 0 https://github.com/rust-bitcoin/rust-secp256k1/blob/master/src/ffi.rs#L25

they're ORed later with the SECP256K1_START_NONE which is actually SECP256K1_CONTEXT_NONE. but I don't think it'll work

@real-or-random
Copy link
Collaborator

real-or-random commented Jun 20, 2019

Have you read the docs? If you don't sign, then you need either a verification context or no context at all. Some functions like PublicKey::mul_assign() that work on public keys need a verification context, others like PublicKey::combine(), don't need a context at all. Functions on secret keys don't need a context. See https://docs.rs/secp256k1/0.12.2/secp256k1/key/index.html .

So playing around with SECP256K1_CONTEXT_NONE is not useful. We handle this internally already. Playing with `ECMULT_WINDOW_SIZE´ makes sense if you need a verification context. But then set it to 2 instead of 4 if you really want the minimal RAM footprint.

And if you need more scalar and group arithmetic than provided by the functions of PublicKey and SecretKey, than this is not the right library.

@vhnatyk
Copy link
Author

vhnatyk commented Jun 20, 2019

But then set it to 2 instead of 4 if you really want the minimal RAM footprint.

I tried that 😊 among first attempt - to no success, maybe I can't allocate chunks bigger then some threshold like 32kb or lower (then it's not documented for MCU)

than this is not the right library.

oh, umm - problem probably I wouldn't find better and more established and reviewed lib for secp256k1 curve

@real-or-random
Copy link
Collaborator

than this is not the right library.

oh, umm - problem probably I wouldn't find better and more established and reviewed lib for secp256k1 curve

Yeah, the only thing I'm saying is that the aforementioned API functions are the only scalar and group operation that this library offers. If you need other operation, we simply don't support them. (You're of course free to fork the project, but then you're on your own.)

@elichai
Copy link
Member

elichai commented Aug 22, 2019

@vhnatyk does #140 solve your problem?

Otherwise I think this should be discussed upstream

@elichai
Copy link
Member

elichai commented Oct 19, 2019

@vhnatyk ping

@vhnatyk
Copy link
Author

vhnatyk commented Oct 19, 2019

@elichai - hi, for two months thoughts that I have to sort it out are bugging me 🤔 , but always something 'urgent'. It should be much easier since secp256k1 #337 got merged in. But haven't sorted it out yet, unfortunately - seems I should be fine with ECMULT_GEN_PREC_BITS = 2 or SECP256K1_CONTEXT_NONE but truth is that I haven't found yet reliably. It has to be something simple - I will definitely have to sort it right before or after SFBW in the next two weeks

@vhnatyk
Copy link
Author

vhnatyk commented Nov 25, 2019

@elichai - very strange, but with lowmemory on 0.16.0 it works. I was looking at PR and there is nothing except

if cfg!(feature = "lowmemory") {
        base_config.define("ECMULT_WINDOW_SIZE", Some("4"));
...

and I tried that right away in build.rs and was trying with 0.15.2 to no success. But now with 0.16.0 everything works just fine 😶 There were few updates to hw since then - so perhaps hw got updated, but I was looking into release notes and nothing regarding mem management there. And I can't downgrade mcu to try. But anyway it works just fine now without any forks. I will try to find out exact reason in background so closing this.

@vhnatyk vhnatyk closed this as completed Nov 25, 2019
@apoelstra
Copy link
Member

Excellent :) I think since 0.15.2 we brought some changes in from upstream that helped, though I don't remember the details.

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

No branches or pull requests

4 participants