Skip to content

Commit

Permalink
Merge pull request #139 from ssbc/disable-recovery
Browse files Browse the repository at this point in the history
Disable exclude crash recovery by default
  • Loading branch information
Powersource committed Aug 31, 2023
2 parents e3ce292 + 89d7098 commit 165967e
Show file tree
Hide file tree
Showing 5 changed files with 80 additions and 73 deletions.
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,8 @@ Makes sure that you're set up to send and receive group invites, by creating an

You can set the secret stack config `config.tribes2.timeoutLow` and `config.tribes2.timeoutHigh` to control how slowly the client should try to fix a conflicting state, where other clients might be trying to fix the same conflict at the same time. The defaults are `5` and `30` respectively, which gives a random timeout between 5s-30s. A higher value reduces the risk of creating new conflicts since other clients don't do the same conflict resolution at the same time, but increase the time that the group is in an unstable state. A lower number corrects things faster but increases the risk of ending up in new conflicts. Should not be `0` or close to it.

You need to set `config.tribes2.recoverExclude` to true (default false) for the above mentioned conflict recovery to happen at all. The recovery is a bit unreliable but might sometimes be needed to repair broken state.

## Security considerations

While we have tried our best to create a secure end-to-end encrypted communication protocol, this module is not fit for use in safety critical situations. Neither the code nor the specification has been vetted by an independent party. Even assuming a solid implementation, and a bug-free spec, we have intentionally left out several security features that are considered state of the art in other apps such as Signal, such as "forward secrecy".
Expand Down
2 changes: 1 addition & 1 deletion index.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ module.exports = {
addMembers: 'async',
excludeMembers: 'async',
publish: 'async',
listMemebers: 'source',
listMembers: 'source',
listInvites: 'source',
acceptInvite: 'async',
start: 'async',
Expand Down
142 changes: 73 additions & 69 deletions listeners.js
Original file line number Diff line number Diff line change
Expand Up @@ -208,84 +208,88 @@ module.exports = function startListeners(ssb, config, onError) {
)

// recover from half-finished excludeMembers() calls
pull(
ssb.tribes2.list({ live: true }),
pull.unique('id'),
pull.map((group) =>
pull(
getPreferredEpoch.stream(group.id, { live: true }),
pull.unique('id'),
pull.drain(
(preferredEpoch) => {
// re-add missing people to a new epoch if the epoch creator didn't add everyone but they added us.
// we're only doing this for the preferred epoch atm
const timeout = randomTimeout(config)
const timeoutId = setTimeout(() => {
reAddMembers(ssb, group.id, null, (err) => {
// prettier-ignore
if (err && !isClosed) return onError(clarify(err, 'Failed re-adding members to epoch that missed some'))
})
}, timeout)
closeCalls.push(() => clearTimeout(timeoutId))

// if we find an exclude and it's not excluding us but we don't find a new epoch, even after a while, then create a new epoch, since we assume that the excluder crashed or something
pull(
getMembers.stream(preferredEpoch.id, { live: true }),
pull.filter((members) => members.toExclude.length),
pull.take(1),
pull.drain(
() => {
const timeout = randomTimeout(config)
const timeoutId = setTimeout(() => {
ssb.tribes2.get(group.id, (err, group) => {
// prettier-ignore
if (err && !isClosed) return onError(clarify(err, "Couldn't get group info when checking for missing epochs to recover"))

// checking if we were one of the members who got excluded now, in that case we ignore this
if (group.excluded) return
const recoverExclude = config.tribes2?.recoverExclude ?? false
if (recoverExclude) {
pull(
ssb.tribes2.list({ live: true }),
pull.unique('id'),
pull.map((group) =>
pull(
getPreferredEpoch.stream(group.id, { live: true }),
pull.unique('id'),
pull.drain(
(preferredEpoch) => {
// re-add missing people to a new epoch if the epoch creator didn't add everyone but they added us.
// we're only doing this for the preferred epoch atm
const timeout = randomTimeout(config)
const timeoutId = setTimeout(() => {
reAddMembers(ssb, group.id, null, (err) => {
// prettier-ignore
if (err && !isClosed) return onError(clarify(err, 'Failed re-adding members to epoch that missed some'))
})
}, timeout)
closeCalls.push(() => clearTimeout(timeoutId))

getPreferredEpoch(
group.id,
(err, newPreferredEpoch) => {
// prettier-ignore
if (err && !isClosed) return onError(clarify(err, "Couldn't get preferred epoch when checking for missing epochs to recover"))
// if we find an exclude and it's not excluding us but we don't find a new epoch, even after a while, then create a new epoch, since we assume that the excluder crashed or something
pull(
getMembers.stream(preferredEpoch.id, { live: true }),
pull.filter((members) => members.toExclude.length),
pull.take(1),
pull.drain(
() => {
const timeout = randomTimeout(config)
const timeoutId = setTimeout(() => {
ssb.tribes2.get(group.id, (err, group) => {
// prettier-ignore
if (err && !isClosed) return onError(clarify(err, "Couldn't get group info when checking for missing epochs to recover"))

// if we've found a new epoch then we don't need to create one ourselves
if (preferredEpoch.id !== newPreferredEpoch.id)
return
// checking if we were one of the members who got excluded now, in that case we ignore this
if (group.excluded) return

createNewEpoch(ssb, group.id, null, (err) => {
getPreferredEpoch(
group.id,
(err, newPreferredEpoch) => {
// prettier-ignore
if (err && !isClosed) return onError(clarify(err, "Couldn't create new epoch to recover from a missing one"))
})
}
)
})
}, timeout)
if (err && !isClosed) return onError(clarify(err, "Couldn't get preferred epoch when checking for missing epochs to recover"))

closeCalls.push(() => clearTimeout(timeoutId))
},
(err) => {
// prettier-ignore
if (err && !isClosed) return onError(clarify(err, "Couldn't get info on exclusion events when looking for epochs that fail to get created"))
}
// if we've found a new epoch then we don't need to create one ourselves
if (preferredEpoch.id !== newPreferredEpoch.id)
return

createNewEpoch(ssb, group.id, null, (err) => {
// prettier-ignore
if (err && !isClosed) return onError(clarify(err, "Couldn't create new epoch to recover from a missing one"))
})
}
)
})
}, timeout)

closeCalls.push(() => clearTimeout(timeoutId))
},
(err) => {
// prettier-ignore
if (err && !isClosed) return onError(clarify(err, "Couldn't get info on exclusion events when looking for epochs that fail to get created"))
}
)
)
)
},
(err) => {
// prettier-ignore
if (err && !isClosed) return onError(clarify(err, "Failed finding new preferred epochs when looking for them to add missing members to or when checking if an epoch is missing"))
}
},
(err) => {
// prettier-ignore
if (err && !isClosed) return onError(clarify(err, "Failed finding new preferred epochs when looking for them to add missing members to or when checking if an epoch is missing"))
}
)
)
),
pull.drain(
() => {},
(err) => {
// prettier-ignore
if (err && !isClosed) return onError(clarify(err, 'Failed listing groups when trying to find missing epochs or epochs with missing members'))
}
)
),
pull.drain(
() => {},
(err) => {
// prettier-ignore
if (err && !isClosed) return onError(clarify(err, 'Failed listing groups when trying to find missing epochs or epochs with missing members'))
}
)
)
}
})
}
6 changes: 3 additions & 3 deletions test/exclude-members.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ const countGroupFeeds = require('./helpers/count-group-feeds')
const Run = require('./helpers/run')
const Epochs = require('../lib/epochs')

const lowTimeouts = { timeoutLow: 0.1, timeoutHigh: 0.2 }
const lowTimeouts = { timeoutLow: 0.1, timeoutHigh: 0.2, recoverExclude: true }

async function getRootIds(peers, t) {
return Promise.all(peers.map((peer) => p(peer.metafeeds.findOrCreate)()))
Expand Down Expand Up @@ -697,9 +697,9 @@ test("restarting the client doesn't make us rejoin old stuff", async (t) => {
test('On exclusion, if we fail to re-add all people, someone else does that instead', async (t) => {
const run = Run(t)
// set alice to be slow to fix mistakes, to allow carol time to do it
const alice = Testbot({ name: 'alice', timeoutScale: 300 * 1000 })
const alice = Testbot({ name: 'alice' })
const bob = Testbot({ name: 'bob' })
const carol = Testbot({ name: 'carol', timeoutScale: 0 })
const carol = Testbot({ name: 'carol', ...lowTimeouts })
const david = Testbot({ name: 'david' })

await run(
Expand Down
1 change: 1 addition & 0 deletions test/helpers/testbot.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ module.exports = function createSbot(opts = {}) {
tribes2: {
timeoutLow: opts.timeoutLow,
timeoutHigh: opts.timeoutHigh,
recoverExclude: opts.recoverExclude,
},
})

Expand Down

0 comments on commit 165967e

Please sign in to comment.