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

Multi-server support & Generate ephemeral pubkeys randomly for each server #20

Merged
merged 3 commits into from Sep 18, 2023

Conversation

scilio
Copy link
Contributor

@scilio scilio commented May 23, 2023

Fixes #9
Fixes #10
Fixes #19

{
"commit": "0899dadc2b75d66d738b7dbfcba4a37460622dcedaf222e688a2a84826eaa1cff1",
"data": [
"d19f914e7a7b5ba1c0ab36d982bd0bc59aa651458a6ee86c2015b57d96424da4a41559069be5ba59e0d2b688f95f1dfb20648e21f9b01ac400cb4879f2ba434c8eb0337d3f58e9e643aa",
Copy link
Member

Choose a reason for hiding this comment

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

Is this entry a typo? If not, could you remind me what this is and how we pick or compute it?

"rangeproof": "None"
}
```
The ephemeral key is randomly generated as
Copy link
Member

Choose a reason for hiding this comment

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

I think we're missing the second ephemeral key 5353ed848b8b2514aa08c8d9a5109ca4ddafe575c07a2a7cb2f19defa58d8442 here.


The `SwapData` structure contains information needed to swap a single output. It has the following fields:

- `excess`: The total excess for the output commitment.
Copy link
Member

Choose a reason for hiding this comment

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

I'm assuming this isn't the total excess. If any of the mixnodes knows the total excess, they can link the inputs and outputs.

- `rangeproof`: The rangeproof, included only for the final hop (node N).
- `input`: The transaction input being spent.
- `fee`: The transaction fee.
- `onion`: The remaining onion after peeling off our layer.
Copy link
Member

@phyro phyro Jul 11, 2023

Choose a reason for hiding this comment

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

Out of curiosity, would it not be simpler to save only the starting onion and the status? Then the first node just needs to peel the layer like any other.

// calculate minimum fee required for the kernel
let min_kernel_fee = TransactionBody::weight_by_iok(0, 0, 1) * fee_base;

add_kernel_and_collect_fees(
Copy link
Member

@phyro phyro Jul 14, 2023

Choose a reason for hiding this comment

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

I remember you mentioned you wanted to make it a single kernel as described here. Is there a reason we don't use this optimization?

@phyro
Copy link
Member

phyro commented Aug 16, 2023

I think we should expose relevant things like crypto, onions, API structs and possibly more as a lib to make it possible for other software (wallet?) to easily use the onion and other functionality.


/// The standard MWixnet server implementation
#[derive(Clone)]
pub struct SwapServerImpl {
Copy link
Member

@phyro phyro Aug 16, 2023

Choose a reason for hiding this comment

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

I'm still battling through this so not all things are clear in my head. I find it a bit complex that we have two different server implementations when the only difference is that one has a facade swap api exposed and stores some things in memory. Was there a specific reason you decided to separate the two and not have Option<SwapImpl> or similar on the mix server?

@phyro
Copy link
Member

phyro commented Sep 9, 2023

@scilio given our limited capabilities to review, I think a good next step would be to make it possible to test this on a testnet. As said before, this would require us to have the ability to use the code to generate hops, onions, comsig on grin-wallet. It's also a good way to test if mixnodes creating their fee outputs works.

@scilio
Copy link
Contributor Author

scilio commented Sep 13, 2023

@scilio given our limited capabilities to review, I think a good next step would be to make it possible to test this on a testnet. As said before, this would require us to have the ability to use the code to generate hops, onions, comsig on grin-wallet. It's also a good way to test if mixnodes creating their fee outputs works.

Integration with grin-wallet is potentially a non-trivial task. I should be able to refactor the onion generation code into its own library now, but I haven't fully wrapped my head around the grin-wallet design enough yet to know how complicated integration would be. I was hoping to explore that idea more and possibly submit a funding request for a milestone 4 to do that integration.

@yeastplume
Copy link
Member

I'm merging this now to make the code easier to work with for a basic wallet integration. PR is still available for review.

@yeastplume yeastplume merged commit 19e12a1 into mimblewimble:main Sep 18, 2023
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.

Generate random pubkey for each payload e2e multi-server tests Inter-server communication
3 participants