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

Add TVER factory check #81

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Add TVER factory check #81

wants to merge 1 commit into from

Conversation

0xAurelius
Copy link
Contributor

Still needs tests, upgrade script, etc.

@0xAurelius 0xAurelius self-assigned this Mar 12, 2025
Copy link

vercel bot commented Mar 12, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
solidity 🔄 Building (Inspect) Visit Preview Mar 12, 2025 8:07pm

Comment on lines 70 to 74
function isValid(address token) internal returns (bool) {
bytes memory tempEmptyStringTest = bytes(ICMARKCreditTokenFactory(C.cmarkCreditFactory()).creditAddressToId(token));
return tempEmptyStringTest.length > 0;
bytes memory cmarkTestString = bytes(ICMARKCreditTokenFactory(C.cmarkCreditFactory()).creditAddressToId(token));
bytes memory tverTestString = bytes(ICMARKCreditTokenFactory(C.tverCreditFactory()).creditAddressToId(token));
return cmarkTestString.length > 0 || tverTestString.length > 0;
}
Copy link
Collaborator

@Atmosfearful Atmosfearful Mar 12, 2025

Choose a reason for hiding this comment

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

Short-circuit evaluation would defer external contract calls until the preceding calls fail. Which saves gas for CMARK retirements (but not for TVER retirements).

function isValid(address token) internal view returns (bool) {
    return bytes(ICMARKCreditTokenFactory(C.cmarkCreditFactory()).creditAddressToId(token)).length > 0 ||
           bytes(ICMARKCreditTokenFactory(C.tverCreditFactory()).creditAddressToId(token)).length > 0;
}

When consulting the AI it also surfaced this:

view Modifier:
Marking the function as view (assuming creditAddressToId is also view) clarifies that no state modifications occur, potentially lowering gas costs when called externally.

And it also taught me something subtle (not relevant, just interesting):

Directly referencing constants can save gas compared to calling a function—if those constants are available in a way that avoids a function call overhead. However, in this case, the getters in C.sol are defined as internal pure functions. With Solidity’s optimizer enabled, these internal pure function calls are usually inlined at compile time. This means that in an optimized build, there’s effectively no gas difference between using the getter functions and referencing the constants directly (if that were possible).

So, while direct constant access might theoretically be marginally cheaper, here the impact is negligible due to inlining by the compiler.

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.

2 participants