[bisq-network/bisq] Upgrade to Gradle 7.3 (PR #5824)

Chris Beams notifications at github.com
Tue Nov 16 13:51:25 CET 2021


> Where is `io.opencensus` (https://github.com/census-instrumentation) and `io.perfmark` (https://github.com/perfmark/perfmark) used?
> I did not find a reference in the source code, so I guess its a transient dependency from some lib (spring?) and should be excluded.

perfmark and opencensus are both transitive dependencies of grpc-core. I remember running across them before, quite a while ago, i.e. in an older version of grpc (they're not new). I would not recommend excluding them. Keep in mind that the move away from gradle-witness to Gradle's built-in dependency verification now captures _all_ dependencies, around 273 as opposed to the just 64 dependencies we were tracking before. So it's not surprising they show up here for the first time.




> I think we should exclude the price node to an independent project as it has very little Bisq dependency which either could be added as gradle dependency there or just refactored away (I think it was just some utils for currency lists).
> That way we would get rid of the spring framework dependencies and probably speed up the build (at least runing price node tests is slow due the real requests).

Sounds like a reasonable change, but I'd recommend making it a subsequent change instead of doing it as part of this PR. I can commit to doing that.

>  Which java version is now required? Does it work already with Java 16? I guess binary build still does not work with that, right? So Java 11 -15 is likely the required version, right?

Building and running against JDKs 11 -> 15 work in conjunction with this PR. JDK 16 and 17 may work, I can't recall whether I tested them. See also #5835.

> Is the grpcVersion, lombokVersion and protobufVersion required?

I can double-check this; one or more of them may not be strictly necessary for the Gradle 7.3 upgrade here, but were rather required for supporting building on Apple M1 (#5835). So even if we didn't upgrade them here, we would definitely have to upgrade them in that PR. For that reason, I'd prefer to leave them as-is here to avoid reworking the PR, dependency verification metadata, etc.



> I prefer to be conservative as long not absolutely needed of if they fix a know bug we encounter. Supply chain attack is a real threat so better to minimize that surface. I doubt that a Google lib has big risks to become victim of that, but a lib like `lombok` has likely less security critical eyes on it.

I agree supply chain attacks are a real risk, but an overly conservative approach to using and upgrading dependencies is also a risk. This concern is something that comes up over and over again, and I think we'd be wise to come up with an 'official' and pragmatic policy on this matter. I suggest something like the following: that like all code changes, dependency updates cannot be committed without review. And that "review" means that the committer making the change needs to account for why the dependencies are being updated and that the reviewer needs to verify those claims. Both parties should consider the context for the changes, including how likely it is that an attack might be involved. For example, a likely scenario for a supply chain attack would be that some helpful-seeming new contributor comes along and informs us that one of our dependencies is out of date, and that we should really use the newest version of it. This would be a highly suspicious change for obvious reasons. On the other hand, the changes in this PR are by nature much lower risk. We're updating dependencies we've been using for years, and we're doing it for our own reasons on our own timeline. The chances of an attacker coordinating a supply chain attack in conjunction with this PR are not zero, but are very low. We should therefore do basic diligence on the dependencies being changed, but we shouldn't be afraid to make the changes that are necessary to keep our build system up to date, that ensure Bisq can be built on relevant architectures, etc.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/bisq-network/bisq/pull/5824#issuecomment-970241093
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.bisq.network/pipermail/bisq-github/attachments/20211116/3b16b379/attachment-0001.htm>


More information about the bisq-github mailing list