docs: create a guide for localnet + MPC node running in TEE setup#1456
docs: create a guide for localnet + MPC node running in TEE setup#1456barakeinav1 merged 32 commits intomainfrom
Conversation
barakeinav1
force-pushed
the
barak/doc/update_localnet_guide
branch
2 times, most recently
from
005f7c8 to
36b4a31
Compare
DSharifi
changed the title
DSharifi
left a comment
DSharifi
left a comment
There was a problem hiding this comment.
Posting my findings so far. The Review is incomplete as I am not able to reproduce.
Overall it's a very nice written guide.
The helper scripts need some more work. I've tried debugging them for a while, but haven't gotten much progress, so let's try going over this tomorrow together @barakeinav1
The scripts are quite fragile, and it requires a lot of manual setting of ports, because the default ports are hard coded on a TDX machine that is shared. Ideally the scripts should try to bind to an available port and expose this instead.
barakeinav1
force-pushed
the
barak/doc/update_localnet_guide
branch
3 times, most recently
from
45218cc to
c7b0aad
Compare
Co-authored-by: Daniel Sharifi <40335219+DSharifi@users.noreply.github.com>
Co-authored-by: Daniel Sharifi <40335219+DSharifi@users.noreply.github.com>
DSharifi
left a comment
DSharifi
left a comment
There was a problem hiding this comment.
These ports need to be updated to correspond with the localnet ports we initialize the contract with
|
Stacked a PR on top with fixes to the Ports and |
barakeinav1
force-pushed
the
barak/doc/update_localnet_guide
branch
from
4c9102d to
2edd694
Compare
netrome
left a comment
netrome
left a comment
There was a problem hiding this comment.
I haven't done a deep review, but from what I can see this looks pretty good. I have some suggestions and thoughts but overall looks good.
There was a problem hiding this comment.
Naturally the same changes as I suggested in frodo.env should be changed here as well.
|
|
||
| #### 4. Replace ${MACHINE_IP} inside the config files | ||
| ```bash | ||
| sed -i "s|\${MACHINE_IP}|$MACHINE_IP|g" ../deployment/localnet/tee/frodo.conf |
There was a problem hiding this comment.
Recommendation: I used envsubst for these kinds of substitutions already in the localnet guide. I think we could use it instead of sed here.
There was a problem hiding this comment.
envsubst can't do inline editing, so it complicates things a bit.
There was a problem hiding this comment.
I prefer generating a new file (e.g. /tmp/localnet/frodo.conf) over inline editing since these files are version controlled and should not be modified in the repo. But sure, not a hard blocker.
There was a problem hiding this comment.
Does this mean that if I follow the guide, a file that is tracked in git is modified, and will need to be reverted back afterwards to avoid committing it by mistake? If yes, then I second the /tmp/... file copy preference, the other way seems a bit annoying for devs.
|
|
||
| --- | ||
|
|
||
| ## Add a Domain |
There was a problem hiding this comment.
This part is the same as in the localnet guide already? Perhaps we could just reference it there? Although I don't mind a little duplication of these commands since they should be very stable and not subject to change.
There was a problem hiding this comment.
yes, it is a duplication, but since this is a small step at the end I think it is ok to leave it there.
4 participants
Create a guide for localnet + MPC note running in TEE setup
fixes #1417