Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(bungee): new skeleton for bungee plugin #84

Closed

Conversation

eduv09
Copy link
Collaborator

@eduv09 eduv09 commented Mar 6, 2024

New architecture for the BUNGEE Plugin for Fabric and Besu networks.

@eduv09 eduv09 self-assigned this Mar 6, 2024
@eduv09 eduv09 changed the title feat(bungee) - Plugin BUNGEE feat(bungee): New skeleton for bungee plugin Mar 6, 2024
@AndreAugusto11
Copy link
Collaborator

@eduv09 nice job! You're getting to grips with this! Just one comment overall:
@RafaelAPB and I are thinking of making a change to the SATP package, which is also applicable here. A gateway is supposed to be a software component that runs within some server that can enable interoperability between whichever networks -- i.e. if a client app needs to transfer an asset between ledgers A and B and then between C and D, both should be possible using the same gateways. This means that there is no Fabric Gateway nor Besu gateway (as we currently have in the SATP package). We just have a gateway that has specific functionality, which is implemented differently when dealing with different chains.

The same though can be applied here for BUNGEE. Our idea is to implement a Strategy Pattern, such that the plugin defines the common functionality (e.g., generateLedgerStates) and each strategy (FabricStrategy, BesuStrategy, etc..) implements the specific function. Therefore, the BesuBungee and FabricBungee plugins would not exist anymore, along with the factory implementation. This should be fairly easy to change given that the code is already split in half, according to the ledger, but let us know if you need further input

P.S. usually we give a bit more information on the commit message (and consequently the PR description) so that everyone knows what this commit is all about without having to figure out everything through the code. Take this commit as an example. And don't forget signing the commit using the -s flag when committing: git commit -s -m ...

1. We had dozens of different versions of ESLint, TSLint, Prettier
declard in package.json files. Most versions were so old that they had
ben EOL for several years too.
2. The presence of these and their scripts/config files/etc were making
it impossible to run the linter on the sub-folders where these were
being used.
3. As part of the big push to consolidate linting and code formatting on
the entire project, this is phase 1 where I purged the mentioned dependencies
from the project and now only the root package.json file declares them
which is currently using the latest and greatest versions.
4. I know that the diff is huge in this pull request but there are no
code changes that would cause bugs or logic modification at all. It's all
just applying automated formatting and linting on sub-directories where
previously this was impossible. E.g., this entire change should not cause
any sort of bugs (but of course we'll see what the CI execution has to
say about that).
5. The follow-up phases will consist of slowly applying more brazen
refactoring in a much more targeted and surgical manner with small
pull requests that are easy and quick to review.
6. The smaller pull requests will target sub-directories that we still
cannot include in the linting (see the .eslintignore file for a list) and
then culminate in us finally enabling the linter for everything.
7. The root folder's ESLint ignore file was refactored in a way that the
files that we plan on fixing later are now ignored on a one by one basis
instead of using patterns. This makes the ignore file a collection of
to-dos where one can delete a single line from the ignore file (pertaining
to a single file that is being ignored) and then proceed to fix the linter
issues and send a small, easy to review pull request that does not overwhelm
the maintainers when they are trying to review a huge diff with a large
batch of linter fixes. Phase two of this project is then to one by one
drain the ignore entries of these mentioned files from the ESLint ignore
file.

So, the bottom line is that this might look like a hugely disruptive
change, but I've done my best to keep it as simple as possible so that
we don't have to worry about reviewing it line by line and instead just
focus on the big picture and the principle idea(s) behind it (code quality).

If it does end up introducing a bug or CI breakage, I'll volunteer to fix
those because I'm very confident that even if this does happen, it will
be very minor and quick fixes necessary only so I don't mind being on the
hook for it.

Signed-off-by: Peter Somogyvari <peter.somogyvari@accenture.com>
No changes other than the mass find & replacing of the GitHub actions
that we use for setting up NodeJS, checking code out and caching.

Address the warnings that started popping up in the CI that look like
this, asking for a mass-upgrade of CI action versions:

> Node.js 16 actions are deprecated. Please update the following actions to use Node.js 20:
> actions/setup-node@v4.0.2,
> actions/checkout@v4.1.1,
> actions/cache@v4.0.1
> For more information see:
> https://github.blog/changelog/2023-09-22-github-actions-transitioning-from-node-16-to-node-20/.

Fixes hyperledger-cacti#3079

Signed-off-by: Peter Somogyvari <peter.somogyvari@accenture.com>
@eduv09 eduv09 force-pushed the Verifiable-private-proof-formats-SATP-1 branch from f04b30a to 456cb83 Compare March 13, 2024 15:36
1. This does not upgrade all the container images that we build and publish
as part of the project, it just upgrades the runner images that are being
used by the continuous integration environment.
2. I also deleted a .yaml file that I left in the diff of another pull request
by accident. Apologies for the littering.
3. Upgrade: as-in, I bumped the 20.04 versions to 22.04
4. Pin: as-in, replaced `ubuntu-latest` with `ubuntu-22.04` everywhere.
5. Reasoning: 24.04 is out now and so we have to keep up with the times
because soon there'll be a stop to security patches even on what used to
be LTS (20.04). It is better to find out now if we have a problem with
ubuntu 22.04 rather than later when it's become urgent to upgrade.
6. The risk of something breaking because of these upgrades is most likely
low since ubuntu is one of the most stable distributions out there.

Signed-off-by: Peter Somogyvari <peter.somogyvari@accenture.com>
@RafaelAPB
Copy link
Owner

This seems great work. Can you please add a README documenting the package with as much detail as possible?

@AndreAugusto11
Copy link
Collaborator

@RafaelAPB what do you think of also adding this to the CI?

Bumps [crypto-js](https://github.com/brix/crypto-js) from 4.1.1 to 4.2.0.
- [Commits](brix/crypto-js@4.1.1...4.2.0)

---
updated-dependencies:
- dependency-name: crypto-js
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <support@github.com>
@eduv09 eduv09 force-pushed the Verifiable-private-proof-formats-SATP-1 branch from 26b6be0 to c73520b Compare March 15, 2024 13:10
@eduv09 eduv09 changed the title feat(bungee): New skeleton for bungee plugin feat(bungee): new skeleton for bungee plugin Mar 15, 2024
1. This fixes many flakes that we were suffering from before with the
older versions of the AIO image.
2. There could still be other flakes lurking around, but their numbers
should definitely be going down with this change due to the increased
stability of the new AIO image.

Signed-off-by: Peter Somogyvari <peter.somogyvari@accenture.com>
@eduv09 eduv09 force-pushed the Verifiable-private-proof-formats-SATP-1 branch 2 times, most recently from 0fc466e to 8396797 Compare March 15, 2024 15:23
petermetz and others added 3 commits March 18, 2024 02:57
1. The default/suggested NodeJS version we use and recommend is v18 LTS
but there were a lot of places in the code that still declared v14 and
v16 both of which are now EOL.
2. This was/is working fine but eventually we'll hit some compiler issue
where the upgrade will be forced so it's most likely wiser to do this little
quality of life/housekeeping change now when it's not urgent.

Signed-off-by: Peter Somogyvari <peter.somogyvari@accenture.com>
Bumps [follow-redirects](https://github.com/follow-redirects/follow-redirects) from 1.14.9 to 1.15.6.
- [Release notes](https://github.com/follow-redirects/follow-redirects/releases)
- [Commits](follow-redirects/follow-redirects@v1.14.9...v1.15.6)

---
updated-dependencies:
- dependency-name: follow-redirects
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>
1. This is a pre-requisite for a bigger change that is adding gRPC
endpoint support to the plugins.
2. We've had gRPC support in the API server for a long time, but the
plugins are not yet able to register their own gRPC services as of now.
3. The reason we need the newer version is because some types are not
exported by the older version that we'll be using to implement the
gRPC support for plugins.

Signed-off-by: Peter Somogyvari <peter.somogyvari@accenture.com>
@eduv09 eduv09 force-pushed the Verifiable-private-proof-formats-SATP-1 branch 3 times, most recently from 33039a2 to f2cebdb Compare March 19, 2024 14:46
petermetz and others added 3 commits March 19, 2024 14:16
1. The solution here was to migrate the image from Debian to Ubuntu
because it seems to not have the same vulnerabilities as the lastest
stable Debian image does, so the change itself is to move to Ubuntu 24.04
LTS.
2. Also upgraded the Rust toolchain to the current latest which mandated
a couple of small code changes that are also added in this commit.

The original security report from Trivy that we've discoverd on the CI:

┌─────────────┬───────────────┬──────────┬───────────────────┐
│   Library   │ Vulnerability │ Severity │ Installed Version │
├─────────────┼───────────────┼──────────┼───────────────────┤
│ libgnutls30 │ CVE-2024-0553 │ HIGH     │ 3.6.7-4+deb10u11  │
│             │               │          │                   │
└─────────────┴───────────────┴──────────┴───────────────────┘
...
┬──────────────────┬───────────────────────────────────────────┐
│  Fixed Version   │                   Title                   │
┼──────────────────┼───────────────────────────────────────────┤
│ 3.6.7-4+deb10u12 │ gnutls: incomplete fix for CVE-2023-5981  │
│                  │ https://avd.aquasec.com/nvd/cve-2024-0553 │
┴──────────────────┴───────────────────────────────────────────┘

Signed-off-by: Peter Somogyvari <peter.somogyvari@accenture.com>
1. User-defined Typescript type-guard function that asserts whether a value or
object is a `@grpc/grpc-js` {Partial<StatusObject>} or not.
The reason why it checks for {Partial} is because all of the properties of
the {StatusObject} are defined as optional for some reason, hence we cannot
assume anything about those being present or not by default.
2. Therefore this method will just check if the `code` property is set or not
and return `true` or `false` based on that.
3. The above is also the reason why the name of the function is slightly more
verbose than your average user-defined type-guard that could be named just
"isGrpcStatusObject()" but we wanted to make sure that more specific type-guards
can be added later that check for other optional properities or for the
presence of all of them together.

Link to the status builder within grpc-js:
https://github.com/grpc/grpc-node/blob/master/packages/grpc-js/src/status-builder.ts

Signed-off-by: Peter Somogyvari <peter.somogyvari@accenture.com>
Signed-off-by: André Augusto <andre.augusto@tecnico.ulisboa.pt>
LordKubaya and others added 13 commits March 20, 2024 11:49
…on 2.0.0 OpenAPI

* created new messages in openAPI definition
Signed-off-by: André Augusto <andre.augusto@tecnico.ulisboa.pt>
Signed-off-by: Rafael Belchior <rafael.belchior@tecnico.ulisboa.pt>

feat(bungee): add skeleton for bungee

Signed-off-by: André Augusto <andre.augusto@tecnico.ulisboa.pt>

refactor(plugin-satp-hermes): update messages formats SATP core version 2.0.0 OpenAPI

* created new messages in openAPI definition

feat(bungee): fix bungee dependencies

Signed-off-by: André Augusto <andre.augusto@tecnico.ulisboa.pt>

feat(SATP-Hermes): gateway coordinator

feat(SATP-Hermes): gateway coordinator

feat(SATP-Hermes): gateway coordinator WIP

Signed-off-by: Rafael Belchior <rafael.belchior@tecnico.ulisboa.pt>

test(SATP-Hermes): enable decorators needed for gateway coordinator checks

Signed-off-by: Rafael Belchior <rafael.belchior@tecnico.ulisboa.pt>

docs(SATP-Hermes): update package.json

Signed-off-by: Rafael Belchior <rafael.belchior@tecnico.ulisboa.pt>

refactor(SATP-Hermes): re-estructure project

Signed-off-by: Rafael Belchior <rafael.belchior@tecnico.ulisboa.pt>

refactor(SATP-Hermes): re-estructure project

Signed-off-by: Rafael Belchior <rafael.belchior@tecnico.ulisboa.pt>

refactor(SATP-Hermes): re-estructure project

Signed-off-by: Rafael Belchior <rafael.belchior@tecnico.ulisboa.pt>

refactor(SATP-Hermes): remove kotlin sdk, add protobuffer

Signed-off-by: Rafael Belchior <rafael.belchior@tecnico.ulisboa.pt>

feat(SATP-Hermes): protobuf structure

Signed-off-by: Rafael Belchior <rafael.belchior@tecnico.ulisboa.pt>

feat(SATP-Hermes): protobuf structure

Signed-off-by: Rafael Belchior <rafael.belchior@tecnico.ulisboa.pt>

refactor(SATP-Hermes): refactor naming

Signed-off-by: Rafael Belchior <rafael.belchior@tecnico.ulisboa.pt>

feat(SATP-Hermes): protobuf structure improvements

Signed-off-by: Rafael Belchior <rafael.belchior@tecnico.ulisboa.pt>

feat(SATP-Hermes): update yarn.lock and clean up

Signed-off-by: André Augusto <andre.augusto@tecnico.ulisboa.pt>

refactor(test-tooling): fix types of streams: use NodeJS.ReadableStream

1. The container management library that we use in the test infrastructure
(called dockerode) is expecting streams that are defined in the global
namespace of the `@types/node` library, e.g. the standard library of NodeJS
itself.
2. Previously we were using the "streams" package to provide type information
to the streams that we were passing around to dockerode and it was working
fine, but after some changes that seem unrelated this has broken the
compilation process.
3. The mentioned changes are not yet on the main branch, but we expect
them to be there soon and so this change is laying the groundwork for that
by pre-emptively fixing the broken build's root cause which is that the
test-tooling package does not declare it's typings related dependencies
correctly: It implicitly uses the NodeJS standard library's types but
so far had not declared them on the package level.
4. This change is therefore to rectify the issue of the `@types/node`
dependency missing from the test-tooling package and also the refactoring
of some of the test ledger classes which were relying on the `streams`
builtin package instead of correctly using the NodeJS.ReadableStream global.
5. Earlier the reasoning for this was that we try to avoid pulling in
types from the global scope because we try to avoid any sort of dependency
on the global scope in general. Once we have proof though that this is
causing issues with the build, then we must give up the principle for
practical reasons (and only in the minimum viable scope, e.g. this does
not change the fact that everywhere else in the codebase we should still
do our best to avoid using the global scoped classes, types, functions,
etc..).

Thank you to @AndreAugusto11 and @RafaelAPB for pointing out this issue
through the pull request of his that is currently being worked on at the
time of this writing:
RafaelAPB#72

Related to but does not address hyperledger-cacti#2811

Signed-off-by: Peter Somogyvari <peter.somogyvari@accenture.com>

refactor(SATP-Hermes): remove unused packages

* updated openapi version
* updated bungee plugin version to 2.0.0-alpha.2

Co-authored-by: Peter Somogyvari <peter.somogyvari@accenture.com>

Signed-off-by: André Augusto <andre.augusto@tecnico.ulisboa.pt>
Signed-off-by: Peter Somogyvari <peter.somogyvari@accenture.com>
Signed-off-by: André Augusto <andre.augusto@tecnico.ulisboa.pt>
Signed-off-by: André Augusto <andre.augusto@tecnico.ulisboa.pt>
- Add SATP-Hermes to CI
- Fix some SATP tests that were hanging the terminal after execution
- Migrated SATP tests to Fabric 2.5.x LTS image

The older Fabric AIO images have become flaky unfortunately due to
auto-upgrades that we can't pin down due to upstream repos doing it.

Co-authored-by: Peter Somogyvari <peter.somogyvari@accenture.com>

Signed-off-by: Peter Somogyvari <peter.somogyvari@accenture.com>
Signed-off-by: André Augusto <andre.augusto@tecnico.ulisboa.pt>
Signed-off-by: Rafael Belchior <rafael.belchior@tecnico.ulisboa.pt>
Signed-off-by: Rafael Belchior <rafael.belchior@tecnico.ulisboa.pt>
Signed-off-by: Rafael Belchior <rafael.belchior@tecnico.ulisboa.pt>
Signed-off-by: Rafael Belchior <rafael.belchior@tecnico.ulisboa.pt>
Signed-off-by: André Augusto <andre.augusto@tecnico.ulisboa.pt>
Signed-off-by: Rafael Belchior <rafael.belchior@tecnico.ulisboa.pt>
* plugin follows strategy design pattern defined in folder strategy/
* current version has strategy for fabric and besu networks
* includes a few tests to demonstrate basic functionality
* added README with package description

Signed-off-by: eduv09 <eduardovasques10@tecnico.ulisboa.pt>
@eduv09 eduv09 force-pushed the Verifiable-private-proof-formats-SATP-1 branch from f2cebdb to 7cf0c90 Compare March 20, 2024 16:23
@eduv09 eduv09 closed this Mar 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants