verification in DMs

[uhoreg]

Fixes element-hq/element-web#10793

implements MSC2241

also cleans up some bits of code, and removes some obsolete (will no longer be used) code

[bwindels]

Looks generally good, just a few things to address before.

[bwindels]

Could use MatrixEvent.getRelation here. We stuck with m.relates_to for now, but if we ever change it to m.relationship, best to have to change it only once.

[bwindels]

I think this would throw if an event comes in before the verification event gets sent (and eventId is defined).
I tried this:

function scope() {
	const f = () => console.log("foo", foo);
	f();
	const foo = 5;
	f();
}
scope();

and it throws with ReferenceError: foo is not defined at the first call of f().

You could put a let eventId at the top and reassign it at the bottom?

[bwindels]

If sendEvent inside this function (or anything after it) would throw, it would get swallowed, as the return value from the promise executor is ignored (here the promise of the async function).

You could instead put the sendEvent after creating this promise (and putting it in a variable) and make the whole method async rather than the promise executor, like so:

const listenPromise = new Promise((_resolve, _reject) => {
...
});
const res = await this._baseApis.sendEvent(...);
return listenPromise;

[bwindels]

I guess you're doing both here because of this discussion? I'd just stick with "m.relates_to" for now, as we're not doing this anywhere else, but feel free to leave it like this.

[uhoreg]

Comments have been addressed.

BK tests passed: https://buildkite.com/matrix-dot-org/matrix-js-sdk/builds/608

[bwindels]

Thanks for the fixes! Looks good, just one nit, but no need to re-review after addressing that, it's good to merge 👍 😄

[bwindels]

nit: this doesn't need to be async anymore i think?