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

Support customized accessor to token whitelist #140

Merged
merged 1 commit into from
Jan 16, 2025
Merged

Support customized accessor to token whitelist #140

merged 1 commit into from
Jan 16, 2025

Conversation

shiatcb
Copy link
Contributor

@shiatcb shiatcb commented Jan 15, 2025

Fixes # .

Motivation

Allow customized token whitelist accessor function to be defined in rosetta-xxx repo.

Solution

See PR

Open questions

@cb-heimdall
Copy link

cb-heimdall commented Jan 15, 2025

✅ Heimdall Review Status

Requirement Status More Info
Reviews 1/1
Denominator calculation
Show calculation
1 if user is bot 0
1 if user is external 0
From .codeflow.yml 1
Additional review requirements
Show calculation
Max 0
0
From CODEOWNERS 0
Global minimum 0
Max 1
1
1 if commit is unverified 0
Sum 1

shell: bash
- uses: actions/checkout@v3

- name: Set up Python
Copy link
Contributor Author

Choose a reason for hiding this comment

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

All other changes are just auto-format except this Setup Python workflow. We need to downgrade to 3.9 to avoid the error when importing web3 library: ethereum/web3.py#2704 (comment)

@jingweicb
Copy link
Contributor

jingweicb commented Jan 16, 2025

LGTM,
one q: how did it test?
especially tests on backward compatibility

@shiatcb
Copy link
Contributor Author

shiatcb commented Jan 16, 2025

LGTM, one q: how did it test? especially tests on backward compatibility

@jingweicb
it's using current unit test to test backward compatibility
The function I modified is called in block_service_test.go via .Block(...)
It checks if the new variable (TokenWhitelistAccessor) is nil, by default it's not defined so it will execute the existing logic

@shiatcb shiatcb merged commit 18b4556 into master Jan 16, 2025
10 checks passed
@shiatcb shiatcb deleted the token branch January 16, 2025 15:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants