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

encoding: Better support for address encodings. #1531

Merged
merged 6 commits into from
May 30, 2023

Conversation

winder
Copy link
Contributor

@winder winder commented May 24, 2023

Summary

There is a bug in the newer version of Indexer used by Conduit which caused addresses to be encoded differently. This PR makes the decoding more lenient to allow for the different encodings.

Test Plan

New unit test.

@winder winder requested review from shiqizng and algochoi May 24, 2023 00:01
@winder winder self-assigned this May 24, 2023
@winder winder force-pushed the will/partupdrmv-override-master branch from 6840b46 to 47c3051 Compare May 24, 2023 15:10
@codecov
Copy link

codecov bot commented May 24, 2023

Codecov Report

❗ No coverage uploaded for pull request base (hotfix/2.15.4@a700bdd). Click here to learn what that means.
The diff coverage is 90.47%.

@@               Coverage Diff                @@
##             hotfix/2.15.4    #1531   +/-   ##
================================================
  Coverage                 ?   65.24%           
================================================
  Files                    ?       52           
  Lines                    ?     8375           
  Branches                 ?        0           
================================================
  Hits                     ?     5464           
  Misses                   ?     2436           
  Partials                 ?      475           
Impacted Files Coverage Δ
idb/postgres/internal/encoding/types.go 66.66% <ø> (ø)
idb/postgres/internal/encoding/encoding.go 89.43% <90.47%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Contributor

@algochoi algochoi left a comment

Choose a reason for hiding this comment

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

Changes look fine but do we need to cherry-pick fixes to this hotfix branch everytime? I'm not familiar with this dual branch system.

edit: Oh is this the legacy indexer? I guess that makes more sense

Copy link
Contributor

@shiqizng shiqizng left a comment

Choose a reason for hiding this comment

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

this looks good. I'm starting an indexer deployment to verify that it works as expected.

@winder
Copy link
Contributor Author

winder commented May 25, 2023

Changes look fine but do we need to cherry-pick fixes to this hotfix branch everytime? I'm not familiar with this dual branch system.

@algochoi This is for the next release. It's a gitflow thing, but also a little strange because of the licensing work currently staged in develop.

Copy link
Contributor

@algochoi algochoi left a comment

Choose a reason for hiding this comment

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

Looks like it was correctly copied

@winder
Copy link
Contributor Author

winder commented May 28, 2023

Confirmed regression fix with mainnet data.

Scenario: Initialize postgres DB using Conduit master branch and a mainnet follower node.

Indexer running the current tip of master: 1c79c90 reproduces the encoding error

$ curl localhost:10000/v2/blocks/20215865?pretty
{
  "message": "error while looking up block for round '20215865': json decode error [pos 641]: failed to decode address 2GpQYYQK6VSxGFumcF/wZ4OLnJDJTASMIG5VL/AyU4k= to base 32"
}

This PR applied to the hotfix/2.15.4 branch: 8d6161b is able to fetch the data.

$ curl -s localhost:10000/v2/blocks/20215865?pretty|grep part
  "participation-updates": {
    "expired-participation-accounts": [

@winder winder merged commit 95bd643 into algorand:hotfix/2.15.4 May 30, 2023
winder added a commit to winder/indexer that referenced this pull request May 31, 2023
@winder winder deleted the will/partupdrmv-override-master branch May 31, 2023 22:27
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