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

Remove epic verifier #2751

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Remove epic verifier #2751

wants to merge 5 commits into from

Conversation

cyberj0g
Copy link
Contributor

Remove Epic verifier and corresponding CLI arguments, because it's replaced with local verification and fast verification based on MPEG-7 signatures.

#2737

CHANGELOG_PENDING.md Outdated Show resolved Hide resolved
@@ -33,8 +33,10 @@ type Fatal struct {
Retryable
}

var ErrTampered = Retryable{errors.New("Tampered")}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why's this being added?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That error moved from epic verifier. Currently, local verification doesn't produce it, but it's used in multiple unit tests, so I left it.

@cyberj0g cyberj0g force-pushed the ip/remove-epic branch 2 times, most recently from 04bfe78 to d047044 Compare February 23, 2023 12:13
@cyberj0g
Copy link
Contributor Author

@hjpotter92 could you please look into test workflow fail on 'Install dependencies' step? It may have something to do with recent EOL of Ubuntu 18.04.

@hjpotter92
Copy link
Member

hjpotter92 commented Feb 23, 2023

@hjpotter92 could you please look into test workflow fail on 'Install dependencies' step? It may have something to do with recent EOL of Ubuntu 18.04.

hey; yes! I've another branch where i'm trying to fix those; but keep hitting this:

GO111MODULE=on CGO_ENABLED=1 CC="" CGO_CFLAGS="" CGO_LDFLAGS="" go build -o "./" -tags "mainnet,experimental" -ldflags="-X github.com/livepeer/go-livepeer/core.LivepeerVersion=0.5.38-efb570dc" cmd/livepeer/*.go
# command-line-arguments
/__t/go/1.17.13/x64/pkg/tool/linux_amd64/link: running gcc failed: exit status 1
/github/home/compiled/lib/libavformat.a(mov.o): In function `mov_read_cmov':
/github/home/ffmpeg/libavformat/mov.c:5241: undefined reference to `uncompress'
/github/home/compiled/lib/libavformat.a(id3v2.o): In function `id3v2_parse':
/github/home/ffmpeg/libavformat/id3v2.c:1019: undefined reference to `uncompress'
/github/home/compiled/lib/libavformat.a(matroskadec.o): In function `matroska_decode_buffer':
/github/home/ffmpeg/libavformat/matroskadec.c:1691: undefined reference to `inflateInit_'
/github/home/ffmpeg/libavformat/matroskadec.c:1706: undefined reference to `inflate'
/github/home/ffmpeg/libavformat/matroskadec.c:1699: undefined reference to `inflateEnd'
/github/home/ffmpeg/libavformat/matroskadec.c:1709: undefined reference to `inflateEnd'
/github/home/compiled/lib/libavformat.a(rtmpproto.o): In function `rtmp_uncompress_swfplayer':
Makefile:51: recipe for target 'livepeer' failed
/github/home/ffmpeg/libavformat/rtmpproto.c:1076: undefined reference to `inflateInit_'
/github/home/ffmpeg/libavformat/rtmpproto.c:1086: undefined reference to `inflate'
/github/home/ffmpeg/libavformat/rtmpproto.c:1104: undefined reference to `inflateEnd'
/github/home/ffmpeg/libavformat/rtmpproto.c:1104: undefined reference to `inflateEnd'
collect2: error: ld returned 1 exit status

make: *** [livepeer] Error 2
Error: Process completed with exit code 2.

it appears to be dependency issue; but have been unable so far to resolve it :(

link: https://github.com/livepeer/go-livepeer/actions/runs/4245335907/jobs/7380710048

@cyberj0g
Copy link
Contributor Author

It either doesn't link with zlib statically anymore on Ffmpeg build for some reason, or it's CGO issue. If zlib is not missing on the system, explicitly adding CGO_LDFLAGS=-lz in Makefile should help.

Not sure it's the same issue as in test workflow, though.

@hjpotter92
Copy link
Member

It either doesn't link with zlib statically anymore on Ffmpeg build for some reason, or it's CGO issue. If zlib is not missing on the system, explicitly adding CGO_LDFLAGS=-lz in Makefile should help.

Not sure it's the same issue as in test workflow, though.

Hey! @cyberj0g thanks for that. adding -lz to make flags was the only pending step.

the builds are now succeeding (had to also trigger a clean cache for darwin builds).

@codecov
Copy link

codecov bot commented Feb 23, 2023

Codecov Report

Merging #2751 (850053a) into master (c65fed9) will decrease coverage by 0.13416%.
The diff coverage is 0.00000%.

❗ Current head 850053a differs from pull request most recent head 9f1f404. Consider uploading reports for the commit 9f1f404 to get more accurate results

Impacted file tree graph

@@                 Coverage Diff                 @@
##              master       #2751         +/-   ##
===================================================
- Coverage   56.28521%   56.15105%   -0.13416%     
===================================================
  Files             88          87          -1     
  Lines          19172       19013        -159     
===================================================
- Hits           10791       10676        -115     
+ Misses          7789        7753         -36     
+ Partials         592         584          -8     
Impacted Files Coverage Δ
cmd/livepeer/livepeer.go 46.77419% <ø> (-0.56624%) ⬇️
cmd/livepeer/starter/starter.go 4.35967% <0.00000%> (+0.07013%) ⬆️
verification/verify.go 89.16667% <ø> (ø)

... and 1 file with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c65fed9...9f1f404. Read the comment docs.

Impacted Files Coverage Δ
cmd/livepeer/livepeer.go 46.77419% <ø> (-0.56624%) ⬇️
cmd/livepeer/starter/starter.go 4.35967% <0.00000%> (+0.07013%) ⬆️
verification/verify.go 89.16667% <ø> (ø)

... and 1 file with indirect coverage changes

@cyberj0g
Copy link
Contributor Author

cyberj0g commented Feb 25, 2023

@hjpotter92 sorry to bother you again, but now it fails with:

Error: .github/workflows/test.yaml (Line: 51, Col: 16):
Error: The template is not valid. .github/workflows/test.yaml (Line: 51, Col: 16): hashFiles('**/install_ffmpeg.sh') failed. Fail to hash files under directory '/home/runner/work/go-livepeer/go-livepeer'

See Trigger test suit job.

@hjpotter92
Copy link
Member

@hjpotter92 sorry to bother you again, but now it fails with:

Error: .github/workflows/test.yaml (Line: 51, Col: 16):
Error: The template is not valid. .github/workflows/test.yaml (Line: 51, Col: 16): hashFiles('**/install_ffmpeg.sh') failed. Fail to hash files under directory '/home/runner/work/go-livepeer/go-livepeer'

See Trigger test suit job.

hey @cyberj0g I had been afk for some time; so couldn't reply. but it looks like the issue was fixed by @mjh1 here: https://github.com/livepeer/go-livepeer/pull/2764/files#diff-245392b692a50c38ecab4381b118862db514035c10983f3bd4f4b7f1f4be4692R51

once you rebase your branch on current master; it should be good to go.

cc @thomshutt @mjh1

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.

3 participants