major rewrite to address resolution/dereferencing confusion. take 2#315
major rewrite to address resolution/dereferencing confusion. take 2#315
Conversation
| verification methods, such as attestations, authorizations, | ||
| encryption, and key agreement. |
There was a problem hiding this comment.
I don't have a concrete suggestion - apologies - but I think we need to distinguish the verification methods from the things the produce (attestions, authorizations etc).
Maybe verification method for purposes such as Feels a bit clunky though.
There was a problem hiding this comment.
Ah... Good catch. These are verification relationships.
|
This was discussed during the #did meeting on 02 April 2026. View the transcriptDID URL Dereferencing PR \[3\] (20 min)<ottomorac> w3c/did-resolution#315 Otto Mora: this week to review this PR, and we will arrive at a special topic call with hopefully more concrete Will Abramson: Yeah, I just wanted to say that's one thing, too, Otto. Maybe, Joe, if you can take, I don't know... Joe Andrieu: Yeah, I'm happy to do that... Will Abramson: I'm sure we could do that. Yeah, great, thanks... Otto Mora: Okay. So... <swcurran> Joe What is the specific item that you said was dismissed -- the multiple resources? I've just lost track. Otto Mora: Yeah, in the interest of time as well, like, yeah, I just want to give Joe time to present his change as well, because I think it is important as well that we understand it Joe Andrieu: Sure, let me… let me speak to it first. There… there are two main changes... Otto Mora: Mm-hmm... Joe Andrieu: And I'm open to all sorts of suggestions on how to clean that up. I'm actually very interested, Manny, in the criticisms you have of it, because if you think I got it wrong, I want to try and fix it... Otto Mora: Okay... Stephen Curran: I… I like... Otto Mora: Hmm... Stephen Curran: works. Um, and so does it require two separate algorithms? Requires a single algorithm?... Otto Mora: Okay, so just to get a range, so did URL... Stephen Curran: The only thing… the only thing you added there was the did doc. The did doc... Otto Mora: Okay... Stephen Curran: That… but, I mean, that I'd be willing to talk about, is did we always also pass back the doc, but I… again, now you're starting to get into passing multiple things back, which gets complicated... Otto Mora: Mm-hmm... <Zakim> JoeAndrieu, you wanted to agree. Dereferencing should start off the specification rather than resolution Stephen Curran: I'm not quite sure how you would handle that... Otto Mora: Okay, thank you. Uh, Joe... Joe Andrieu: Um, yeah, it's interesting. The way you described it is how I would describe the current algorithm. Um... Otto Mora: Mm-hmm. So, I guess, did URL goes to either dereferencing or resolution?... Joe Andrieu: No, no. The… what you do with a URL is you dereference it to bring it in the current context so that... <swcurran> Absoluttely a DIDDoc must be resolved -- just a question of whether it is the result to the client Joe Andrieu: Um, either the browser can display it, or you can pull it into a JavaScript process and use the data that came back from that Otto Mora: Okay, got it. So they… yeah, the referencing filter first, the pointer resource, then... <ottomorac> Wip Otto Mora: If it is indeed something to be resolved, then resolution applies. Okay. Thank you. Um… Uh, yes, Will Will Abramson: Yeah, I don't know if Joe will like this, but I wondered about... <manu> I'm not on board with that :) Will Abramson: I feel like people are on board with that, like, is there a smaller PR that could do that rearranging, maybe update the introduction like you proposed? Otto Mora: Yes... Manu Sporny: Uh, yeah, so let me kind of point out the things I like about the PR. Um, I like the fact that it is breaking apart things that were previously together, right? Uh, I don't think we should have just one long algorithm, uh, you know, to do any one of the things that we're doing. So, plus one... Otto Mora: Okay, Steven?... Stephen Curran: Um, I'm gonna… I would prefer to see one long algorithm... <ottomorac> Joe will have final word <manu> I would prefer to NOT see one long algorithm. Stephen Curran: Um, because I think it makes more sense. I… I do think we need to… I keep bringing up the fact that the spec currently says there's 5. Possible outcomes? <TallTed> One long algorithm might often be decomposable into several internal sub-algorithms... Stephen Curran: from, uh, from the DID that is… the DID URL that's being processed. And again, I always use DID URL because, again, even if you're just getting the DID, because of version time and version Otto Mora: Mm-hmm. Okay, uh, Joe is gonna have the final word, but I… during the special topic call, we will dedicate equal time to both of these, uh, pull requests... Joe Andrieu: Thanks. Um... <swcurran> +1 Joe Andrieu: um, some version of the algorithm you're trying to define, like, into this PR, and I did not attempt to do that <swcurran> Agreed Joe Andrieu: I just tried to capture what was already in the spec with regard to relative ref, and how service parameters are treated happen Otto Mora: Okay. Okay... Joe Andrieu: Wait, I'm sorry, I forgot, I had, um, a couple of other things. Um, I had two index cards, my apologies... <manu> You can make normative changes in CR, even giant ones. Joe Andrieu: But we are proposing that there are normative steps that the client of Resolver should go through before and after. Um, and I think also this speaks to that there are not, in my PR, 5 possible outcomes <Wip> +1 definitely <swcurran> That's what the client does with the result. Not with the dereferencing returns. Joe Andrieu: And so, if I have a did method, and I know that I need a resolver that supports a particular interface Otto Mora: Okay. Uh, just one thing here on the timing for next week. So, Will, I see that the meeting is scheduled for... Will Abramson: That's the time it is. Well, we... Otto Mora: At the moment, it is… It needs to be… it needs to be the same time, right?... Will Abramson: Yeah, I mean, we could try and find another time, but that's just been the time that we call. We've… been at, it feels like... Otto Mora: So it… it needs to be 8am Pacific, right?... Will Abramson: 7am Pacific is when we have always... Joe Andrieu: Yeah, why do you... Otto Mora: Oh, we've always had it. Oh, okay, just making sure... Joe Andrieu: No, we've always had it at 7. Yeah, it's an annoying time for... Otto Mora: Okay, okay, okay, sorry, yes... Joe Andrieu: For some of us, but... Otto Mora: Okay. Alright, well, then that's it... Will Abramson: I will say, I think the transcriber bot is a success, right? Like, it's… I mean, I've not noticed, I've not, like, paid too detailed attention, but it seemed to work much better than last month... <ottomorac> transcriber-bot, pause Pierre-Antoine Champin: It was much better, yeah... |
|
If I'm reading this correctly, it seems "relativeRef" is only mentioned in the "Service Selection Algorithm", not in the "Service Type Selection Algorithm"? Also, I wonder about this sentence:
Does this mean that "relativeRef" is only supported in conjunction with PathService, not with "service" and "serviceType" parameters? |
|
Why does this remove the "expandRelativeUrls" and "verificationRelationship" options? Is this intentional or an accident? |
Co-authored-by: Will Abramson <[email protected]>
Co-authored-by: Will Abramson <[email protected]>
That's an oversight. We should probably apply that in both cases. |
Because I got rid of the "dereferencer" as a library and the resolver should return the authoritative DID document. I'd be on board with defining these algorithms for a resolver client to apply after getting the DID document from resolution, but in this proposal, the software formerly known as a dereferencer is the resolving client and they don't have to return anything. They bring the resource into the current context, as is appropriate for that context. It's only because there's a "function" in the current spec that it needs to return things. But I think it is important to clarify what a resolving client does without requiring it to support external interfaces it has no reason for. Just like we don't require browsers to expose a dereferencing function for URLs. They just dereference into current context, as appropriate. CSS links get added to the CSS object model for the HTML page that has the link. Images are displayed. JavaScript elements are parsed and invoked. What is done with the resource is up to the client and trying to squeeze that through an interface is just confusing and weirdly limiting. |
|
@jandrieu I still don't get why you think that the current specifcation defines a dereferencer as a library or a piece of software that needs to "expose" something. Dereferencing is a process/function which returns a result. The word "return" is meant in an abstract/logical way, just like the function of adding 1+1 "returns" the result 2. This doesn't mean that there is an external endpoint. When a browser dereferences an HTTPS URL including a fragment, then that dereferencing process/function also "returns" something. If you look e.g. at Figure 10 of the current specification, it shows exactly what everybody is saying all the time, i.e. that dereferencing the fragment is done on the client side. I think this is exactly the same idea that you call "dereference into current context". So I really don't understand where your confusion or desire for such major rewrites comes from? |
Co-authored-by: Will Abramson <[email protected]>
Co-authored-by: Will Abramson <[email protected]>
Because the spec requires that dereferencers expose a function with specific inputs and outputs. Section 5 DID URL Dereferencing defines a function which MUST be implemented and MUST implement a particular interface.
This is the heart of the disagreement. As my suggested PR puts it,
Dereferencing is an algorithm, which may affect the current context in any number of ways. Requiring that it return a content stream leaves the actual integration of that string into the current context to an underspecified "client" which is rather confusingly described:
This language implies the "client" == the software calling the resolver (which would be the dereferencer, NOT the client calling dereferencing. So, where do we define the client that calls the dereferencer? I believe this is the heart of the confusion about dereferencing versus resolution. It's a strange anti-conflation between clients and dereferencers, which should be the same but are confusingly given different tasks at different times.
I believe we've had some consensus that the resolver does not return the resource (except in the degenerate case where the resource is the DID document, but the caller doesn't know yet if that's the final resource without further evaluation, so its more accurate to say the resolver returns a DID document which may or may not be the final resource). So "dereferencing the resource" is definitely not what the resolver does. It returns the authoritative DID document. Getting the actual resource is the job of the client of the resolver, not the resolver. In fact, section 5.4.1 explicitly describes what the dereferencer must do, NOT the resolver.
So, if your prior statement were correct, it would be in conflict with Section 5.4.1 which clearly defines [Dereferencing the Resource] as something the DID URL dereferencer does. This is inconsistent and confusing.
Respectfully, it is described much more explicitly as a function that MUST be implemented with a particular signature and responses. That's the problem. That definition, which is required for all conformant "dereferencers" does not fully dereference the URL into the current context, it returns a content stream that the client--who is actually dereferencing a DID URL into the current context--who must finish dereferencing. So the function as defined doesn't actually finish dereferencing, which is left to the "client" of the dereferencer. This is confusing.
Yes, dereferencing the fragment is done by the client. So a fundamental portion of dereferencing cannot be achieved by the dereferencer. This is confusing. Why name a piece of software as a dereferencer if it doesn't actually dereference into the current context?
Hopefully my previous comments highlight specific instances of confusion. However, my rewrite is not from a desire to do so, but a commitment to answer Jeffrey's question from the TAG review. First, Jeffry Yaskin in the TAG review:
In fact, I do think we mean resolution in the meaning in RFC3986 as referred to by Jeffrey. But our discussion of it is confusing. So I took on responding to Jeffrey. I had hoped that would be a simple clarification of a few sentences here and there, but once I got into it, I realized that the best way to get my concerns across was to present a fully-formed alternative. Second, Stephen Curran, on multiple occasions has argued that the distinction between dereferencing and resolution doesn't make any sense to him and that we should just use the same term for both. That one of our own struggles to distinguish between the two is a fairly clear sign that our language isn't clear. It doesn't tell us how to fix it, but it does point to a deep conceptual divide in our own community. Third, I tried to help Stephen separate dereferencing from resolution in PR #260. First commenting piecewise on the language in those PRs. Then, at his request, I attempted a rewrite to give him some language that would address my concerns while trying to support the feature he is trying to get into the spec. I failed. I got through an update of the initial section that introduced definitions for PathService and Relative Ref, but once I started looking at the dereferencing algorithm, I too found that it was too confusing to integrate Stephen's algorithm in a way that wouldn't break existing practice. So, after Jeffrey, Steven, and I were unable to make sense of how the current specification handles the distinction between dereferencing and resolution, I first confirmed my understanding of the algorithm, using the context of the DID Resolution Threat Model to get feedback on the process as I understand it. #226 (comment) and #226 (comment) Then I turned my attention back to the spec and realized that the algorithm as I understood it was, in fact, not the algorithm defined in the spec, which introduces a required component called a dereferencer, which paradoxically, can't actually dereference into the client context. Ah-hah! I believe that this is where the confusion is. If you have to dereference the result of the dereferencer into the current context, then why is the dereferencer called a "dereferencer" and what is it really doing? I raised this as issue #310, which so far only you, Markus have responded to, with a very reasonable concern that it is late in the timeline to consider such far reaching changes. Unfortunately, I don't know how to respond to Jeffrey's (and Stephen's and my) confusion without a revision that more clearly distinguishes the phases in the dereferencing algorithm and restores the client as the dereferencer and removes the intermediary that seems to be the root of the confusion. So I provided spec text that I believe clears up the confusion. This is a big refactoring, both in substance (removing the dereferencer) and in structure (making the algorithm clearer at the top level with better separation between stages). It is late in the game to consider it. But we didn't realize how problematic it was until Jeffry and then Stephen brought the conflict to our attention. This PR is simply my best effort to resolve the confusion that the current spec creates. |
| Some DID parameters are completely independent of any specific | ||
| <a>DID | ||
| method</a> and intended to function the same way for all | ||
| <a>DIDs</a>. Other DID parameters | ||
| are not supported by all <a>DID methods</a>. Where optional | ||
| parameters are | ||
| supported, they are expected to operate uniformly across the | ||
| <a>DID methods</a> | ||
| that do support them. The following table provides common DID | ||
| parameters that SHOULD function the same way across all <a>DID | ||
| methods</a>. Support for all | ||
| <a href="#did-parameters">DID Parameters</a> is OPTIONAL. |
There was a problem hiding this comment.
| Some DID parameters are completely independent of any specific | |
| <a>DID | |
| method</a> and intended to function the same way for all | |
| <a>DIDs</a>. Other DID parameters | |
| are not supported by all <a>DID methods</a>. Where optional | |
| parameters are | |
| supported, they are expected to operate uniformly across the | |
| <a>DID methods</a> | |
| that do support them. The following table provides common DID | |
| parameters that SHOULD function the same way across all <a>DID | |
| methods</a>. Support for all | |
| <a href="#did-parameters">DID Parameters</a> is OPTIONAL. | |
| Some DID parameters are completely independent of any specific | |
| <a>DID method</a> and intended to function the same way for all | |
| <a>DIDs</a>. Other DID parameters are not supported by all | |
| <a>DID methods</a>. Where optional parameters are supported, | |
| they are expected to operate uniformly across the <a>DID | |
| methods</a> that do support them. The following table provides | |
| common DID parameters that SHOULD function the same way across | |
| all <a>DID methods</a>. Support for each and every | |
| <a href="#did-parameters">DID parameter</a> is OPTIONAL. |
|
|
||
| <section id="dereferencing"> | ||
| <h1>DID URL Dereferencing</h1> | ||
| <h1><dfn data-lt="DID URL Client|Client">DID URL Client</dfn></h1> |
There was a problem hiding this comment.
I think this "DID URL Client" algorithm is significantly under-specified such that an implementers will not know what to do. The following are a few examples that I think are problematic:
- Combinations of service and serviceType parameters. For example, as specified serviceType parameters are silently ignored if a service parameter exists.
- I don't see what happens when a service or serviceType is "selected"? It’s not clear to me what is the result of that selection.
- Per the algorithm, if a property in the DIDDoc or DIDDoc Metadata is found that relates to dereferencing it should be used — without any trigger to indicate it should be used. I would interpret that as, if I pass in a DID URL that is “just a DID”, and I find a Linked Resource in the retrieved DID Doc, I am supposed to return the resource it points to. I’m sure that is not what is meant, but that is how I read it.
- I don’t see that anything is defined if any other query parameters are encountered, or if a path is included.
While these are examples, I don't think this algorithm is backwards compatible with the existing spec (is that a goal?) and is incomplete.
In general, I think this algorithm is a step back from what is in the spec today, making implementations harder.
There was a problem hiding this comment.
I think this "DID URL Client" algorithm is significantly under-specified such that an implementers will not know what to do.
That may still be true. However, I contend that it is far clearer what to do in this PR than in the current specification.
The following are a few examples that I think are problematic:
- Combinations of service and serviceType parameters. For example, as specified serviceType parameters are silently ignored if a service parameter exists.
That's an interesting question. My personal opinion is that specifying both is overspecifying and I would return an error. @wip-abramson suggested maybe the better interpretation is that if both are used that a consistency check is applied such that if the service type doesn't match the service object specified by the service parameter, return an error.
What we should NOT do is simply return all the URLs for all the matching objects. How would the client understand which one was for which?
My algorithm has a specific prioritization. It could probably be improved, but IMO, if a service is named in a service parameter, that should define retrieval. If that service name fails to find any objects, the dereferencing should fail. A clear deterministic algorithm will simplify implementations. An algorithm that tries to waffle on how to prioritize will not.
- I don't see what happens when a service or serviceType is "selected"? It’s not clear to me what is the result of that selection.
From the PR:
The retrieval strategy is determined by the type property of the selected service.
This is explicitly stated in the Section 6.3 Determine Retrieval Strategy steps 1 and 2.
- Per the algorithm, if a property in the DIDDoc or DIDDoc Metadata is found that relates to dereferencing it should be used — without any trigger to indicate it should be used. I would interpret that as, if I pass in a DID URL that is “just a DID”, and I find a Linked Resource in the retrieved DID Doc, I am supposed to return the resource it points to. I’m sure that is not what is meant, but that is how I read it.
That is what is meant. We have an unresolved question about how the client knows which properties have that feature. My proposal is not in this PR, but I have proposed it in yours, Stephen. Define a type property that labels an object as a path handler.
If there's support for that, I could add it.
In fact, I have said many times that supporting the actual functionality you want @swcurran is going to be easier in this PR than in yours, which breaks linkedResources in ways that I still haven't succeed at explaining to you. I created that property. I know why it was designed the way it was, and there simply is no way to return a URL, did or otherwise, for all of the variants that linkedResources supports.
- I don’t see that anything is defined if any other query parameters are encountered, or if a path is included.
Indeed. I don't know of any impact on dereferencing from any other query parameters. The ones defined in the spec like versionId and versionTime do not affect dereferencing, only resolution. Personally, I would rather NOT open up the dereferencing algorithm to dynamically change based on arbitrary query parameters other than service and serviceType. We have enough of a challenge handling the arbitrary properties that have their own dereferencing algorithms.
So, I didn't include it as I know of no approach to affecting dereferencing through query parameters other than service and serviceType. And I strongly think it would be a bad idea to extend beyond that. If we have evidence of practices in the wild that do in fact, do that, then I might be convinced, but please, let's not add complexity for some notion of consistency.
While these are examples, I don't think this algorithm is backwards compatible with the existing spec (is that a goal?) and is incomplete.
I'm sure it can be made more complete. I don't pretend that it is done. This is literally the first pass at a MAJOR change and if the group doesn't want to go in this direction, there's no need to improve it.
So, no, it was not designed to be compatible with the existing spec. It was designed to clear up the confusion that the existing spec has created. For you. For Jeffrey. For me. And, I believe, for a lot of folks.
Forcing dereferencing into a function call is a category error that, IMO, is adding exceptional complexity and confusion without actually creating any clear features or functions we don't already get by defining a coherent algorithm the client uses.
In general, I think this algorithm is a step back from what is in the spec today, making implementations harder.
It may make them harder, but I think it makes them clearer. What is hard is figuring out what to do with the five different kinds of results you might get from the dereferencer and even harder to figure out how to dereference a URL whose method you don't understand. And that latter part is, IMO, the entire point of the DID resolution spec: to allow arbitrary clients to get a DID document they can reason over within their particular context, regardless of the DID method. Once the client software needs to do things differently because of the DID method, we fail at interop.
I would welcome a PR from anyone else if folks see an easier way to respond to Jeffrey's confusion. At this point, it seems that I'm the only one who has been willing to take that on and it is more than frustrating to have my work attacked because it changes things. The POINT is to clear up the confusion. That means changes. Based on my read of the spec, that dereferencer is the cause and change is called for.
If you think otherwise, anyone, please suggest an alternative PR that you feel will clear up Jeffrey's concerns.
|
I’m not a fan of how the term “client” is used in this PR. In my mind, the goal of this specification is that a DID URL can be passed to different implementations of this specification, and the result returned will be the same from each. I think of the “client” is the thing that calls those implementations, and what gets back what it expects based on the DID URL it passed in and then using it in some way — via business logic that is outside the scope the specification. While I have struggled with the term “DID URL Dereferencer”, I think it separates it from client and should be retained. I would prefer not use the term “client” as it is used in this PR. |
wip-abramson
left a comment
There was a problem hiding this comment.
Currently there are a number of ReSpec linking inconsistencies that should be addressed.
At least the instances where the type of ReSpec linking used appears incorrect. E.g. a biblio link instead of a section
Not exactly. If another service type wants to use relative ref, that's fine too. I was trying to split the difference with @swcurran's PR and support the only service type I know of that uses relative ref. It certainly is the only one proposed so far to be defined in this specification. What should happen if relative ref is defined and there is no service endpoint known to use that query parameter? Happy to update if we figure out how we want to handle that. Another thing came up when @wip-abramson and I discussed this. One consequence of this update to the algorithm is that it becomes exceptionally clear that when dereferencing a DID URL with a service or serviceType query parameter, that the service definition needs to define a retrieval strategy. And none of the service types I know adequately address this. There was a long held assumption by many of us--and I also bought into it--that a service is likely derereferenced by applying a GET request of the http URL in the service_endpoint property. But that sort of tribal assumption is one of the things we need to formalize when we define a spec like this one. That approach doesn't work if the service endpoint isn't an HTTPS URL. You can see this is the challenge with the bitcoin: scheme used in the endpoints for BTCR2. A "standard" dereferencing of that endpoint is just going to hurl. If fact, according to BIP 21 https://en.bitcoin.it/wiki/BIP_0021 that URI is used for making bitcoin payments and when dereferenced in a wallet brings up a payment form prefilled with that bitcoin address. This is clearly not how you retrieve the data from a Beacon as defined in BTCR. The BTCR2 specification doesn't talk about how to dereference a DID URL with a service query for a beacon (from example at https://dcdpr.github.io/did-btcr2/data-structures.html#initial-did-document-example-panel-show) : In theory that SHOULD dereference to something, but what is undefined. Instead, that URL is actually used as a URI by the resolution algorithm and is really, only ever used when validating the chain of updates as described in resolution. Its "retrieval strategy" when accessed directly is undefined. Which is, IMO, the right boundary. If a dereferencing client gets a URL with that service, but it doesn't know how to retrieve the resource for a "SingletonBeacon" it should error out. |
Yes. We share that goal. But the term "client" is not one I created. It was already in the specification, underdefined as just the "client of the resolver". A URL (did or otherwise) can be passed to any application that wants to consume it, but what actually happens with it is entirely context-dependent. A URL in an RDF statement is used differently than a URL in a hyperlink is different than a URL in a Verifiable Credential. Even within the browsing context, what is expected to be consistent is that all browsers will attempt to render the same resource (or really a representation of a facet of that resource) by querying the same authority and treating its response as authentic. But different browsers handle different media types differently. Some browsers--like Brave--intentionally do NOT dereference many of the tracking mechanisms that most browsers blindly process, which absolutely breaks many web pages, even when Brave is displaying exactly the same HTML as another browser. The commitment is not the equivalence of the rendering, but the equivalence of the network request to retrieve a representation of the resource from a server specified by the authority (which includes CDNs and round-robin DNS approaches).
Yes. That's exactly what the client is. The client, which is the software that calls the resolver, is the one that processes the business logic to apply the result of resolution to the current context. There is no dereferencer needed in that flow and plenty of reasons it creates problems, which I have enumerated. |
peacekeeper
left a comment
There was a problem hiding this comment.
In general I like the idea of making the dereferencing process more modular, but for now:
-
As I mentioned in #315 (comment), I think this PR breaks how service, serviceType, and relativeRef work at the moment.
-
Also, several of my comments in #311 have not been addressed and still apply here in this PR.
-
I disagree with removing several features such as verificationRelationship and expandRelativeUrls, which we have extensively discussed in the past and decided to include.
-
This removes the dereference() function, and I worry that this might break the use cases I listed in #306 (comment).
I'll take a look at these in more detail and attempt to address what I can. |
Co-authored-by: Ted Thibodeau Jr <[email protected]>
Co-authored-by: Ted Thibodeau Jr <[email protected]>
Co-authored-by: Ted Thibodeau Jr <[email protected]>
Co-authored-by: Ted Thibodeau Jr <[email protected]>
Co-authored-by: Ted Thibodeau Jr <[email protected]>
Co-authored-by: Ted Thibodeau Jr <[email protected]>
Co-authored-by: Ted Thibodeau Jr <[email protected]>
Co-authored-by: Ted Thibodeau Jr <[email protected]>
Co-authored-by: Ted Thibodeau Jr <[email protected]>
Co-authored-by: Ted Thibodeau Jr <[email protected]>
Co-authored-by: Will Abramson <[email protected]>
Replaces PR #311
I believe I have responded to all of the comments and suggestions in that PR.
Hopefully this one is easier to compare.
FWIW, I would move the dereferencing section ahead of the resolution section, as per my comment. However, since people complained about not being able to tell where the changes were, I reverted that edit. Otherwise, both sections would look like a complete rewrite.
Preview | Diff