Fix OpenaiChat provider, improve proofofwork #1974
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull Request Review
Summary
The pull request introduces several changes to the OpenaiChat provider and related files. The main focus seems to be on improving the proof of work mechanism and handling authentication requirements more robustly.
Code Review
- The addition of
max_retriesas a parameter increate_async_generatoris a good approach to handle transient errors that may occur during API interactions. - Refactoring the proof of work and authentication token generation into the retry loop is a smart move, ensuring that each attempt has a fresh set of credentials.
- The global state change for
proofTokensinhar_file.pycould potentially lead to side effects. Consider encapsulating this state within a class or a context manager for better isolation. - The removal of the proof token cache in
proofofwork.pysimplifies the code, but ensure that this does not impact performance significantly due to the lack of caching.
Suggestions
- It would be helpful to include comments explaining the rationale behind significant changes, especially for the refactoring of authentication and proof of work handling.
- Consider adding unit tests to cover the new retry logic and ensure that the proof of work generation behaves as expected under different conditions.
Overall, the changes are well thought out and address some of the complexities involved in managing authentication and proof of work for API interactions. Great work!
Thank you again for your efforts, and looking forward to seeing these improvements in action!
Best,
g4f Copilot
|
|
||
| def generate_proof_token(required: bool, seed: str = None, difficulty: str = None, user_agent: str = None, proofTokens: list = None): | ||
| def generate_proof_token(required: bool, seed: str = "", difficulty: str = "", user_agent: str = None, proofTokens: list = None): |
There was a problem hiding this comment.
The default values for seed and difficulty have been changed to empty strings, which alters the function's behavior. Ensure this change aligns with the intended use cases.
|
|
||
| if proofTokens: | ||
| config = random.choice(proofTokens) | ||
| config = proofTokens[-1] |
There was a problem hiding this comment.
Selecting the last element of proofTokens may not be random as the previous implementation. If randomness is required, consider maintaining that feature.
| config[7] = random.randint(101, 2100) | ||
|
|
||
| diff_len = None if difficulty is None else len(difficulty) | ||
| diff_len = len(difficulty) |
There was a problem hiding this comment.
This line will raise a ValueError if difficulty is an empty string, which is now a possible default value due to the change on line 8.
| for i in range(100000): | ||
| config[3] = i | ||
| json_data = json.dumps(config) | ||
| base = base64.b64encode(json_data.encode()).decode() | ||
| hash_value = hashlib.sha3_512((seed or "" + base).encode()).digest() | ||
| hash_value = hashlib.sha3_512((seed + base).encode()).digest() |
There was a problem hiding this comment.
Concatenating seed and base without a delimiter may lead to unexpected hash values. Consider adding a delimiter between them.
| return "gAAAAAC" + base | ||
| proof_token_cache[seed] = "gAAAAAB" + base | ||
| return proof_token_cache[seed] | ||
| if hash_value.hex()[:diff_len] <= difficulty: |
There was a problem hiding this comment.
The condition hash_value.hex()[:diff_len] <= difficulty assumes difficulty is a hexadecimal string. Validate that difficulty adheres to the expected format.
No description provided.