Skip to content
This repository has been archived by the owner on May 13, 2022. It is now read-only.

Incentivise Network Diversity #362

Closed
wants to merge 1 commit into from

Conversation

adlai
Copy link
Contributor

@adlai adlai commented Dec 14, 2015

This commit makes naive sybil attacks more difficult by keying counterparty
deduplication off IRC hostmasks, disincentivising running multiple yield
generators from the same machine or network.

It also fixes a minor bug: the transaction initiator must deduplicate BEFORE
counting the number of liquid counterparties.

It also tidies a bit of suboptimal yapfage, and factors out a rhyming utility.

It is a revival of #313, neé #311

@adlai
Copy link
Contributor Author

adlai commented Dec 14, 2015

It should be noted that yield generators using the hidden service will be at a disadvantage when transactions are initiated by users of this code, since liquidity providers are incentivised to use unique IPs. If anything, this is a net benefit, because connecting via the hidden service incurs higher latency without tangible privacy benefit: hidden services conceal the location of the server, but provide no better privacy for clients.

@@ -290,7 +293,7 @@ def choose_sweep_orders(db,
txfee,
n,
chooseOrdersBy,
ignored_makers=None):
Copy link
Member

Choose a reason for hiding this comment

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

Comment also applies to L305-307.

See:

ignored_makers=None):
'''
choose an order given that we want to be left with no change
i.e. sweep an entire group of utxos
solve for cjamount when mychange = 0
for an order with many makers, a mixture of absorder and relorder
mychange = totalin - cjamount - total_txfee - sum(absfee) - sum(relfee*cjamount)
=> 0 = totalin - mytxfee - sum(absfee) - cjamount*(1 + sum(relfee))
=> cjamount = (totalin - mytxfee - sum(absfee)) / (1 + sum(relfee))
'''
if ignored_makers is None:
ignored_makers = []

@ghtdak explained to me that this change is due to a 'feature' in Python; see: http://docs.python-guide.org/en/latest/writing/gotchas/#mutable-default-arguments

I suggest reverting these changes.

@adlai adlai force-pushed the sorry-sybil branch 2 times, most recently from eeff894 to b5e7951 Compare December 15, 2015 07:32
@adlai
Copy link
Contributor Author

adlai commented Dec 15, 2015

As discussed on IRC:

  adlai | deduping counterparties by hostmask messes up the tests, since all the scripts are on the same host
  adlai | it works fine on an actual server with distinct connections
waxwing | that's a very interesting example of a test failing because the code works correctly
  adlai | my reflex is to just include our own diddled miniircd, for testing
waxwing | adlai: probably, yes, we could keep it in the test folder. i get your point: you might need to diddle the server code to, what, assign random hostmasks?
  adlai | just append host[1] (ie, source port) in https://github.com/jrosdahl/miniircd/blob/ca46821/miniircd#L128

So this PR is pending: fixing this test, alerting in the forum, and an 🆗 from @chris-belcher.

adlai added a commit to JoinMarket-Org/miniircd that referenced this pull request Dec 15, 2015
adlai added a commit to JoinMarket-Org/miniircd that referenced this pull request Dec 15, 2015
@chris-belcher
Copy link
Collaborator

I agree in principle I guess. Although its trivial for an attacker to get around this, at least they have to work for it a bit.
I'm slightly nervous about this being cyberguerilla specific. Other IRC servers might not allow tor connections from exit nodes.

I'd say merge but when the multiirc branch is finalized maybe have some check to make this only fire for cyberguerilla. I haven't even started looking for other IRC networks to use so I don't know how multiirc will end up looking like. The multiirc branch probably makes this kind of attack easier anyway.

This commit makes naive sybil attacks more difficult by keying counterparty
deduplication off IRC hostmasks, disincentivising running multiple yield
generators from the same machine or network.

It also fixes a minor bug: the transaction initiator must deduplicate BEFORE
counting the number of liquid counterparties.

It also tidies a bit of suboptimal yapfage, and factors out a rhyming utility.
@adlai
Copy link
Contributor Author

adlai commented Dec 17, 2015

Rebased onto develop (e0f7c59)

@adlai adlai closed this Dec 31, 2015
@adlai adlai deleted the sorry-sybil branch December 31, 2015 17:38
AdamISZ pushed a commit to JoinMarket-Org/miniircd that referenced this pull request Dec 1, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants