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

veSugar.all() fails with web3.py #105

Open
Nikotos opened this issue Jan 7, 2025 · 9 comments · May be fixed by #107
Open

veSugar.all() fails with web3.py #105

Nikotos opened this issue Jan 7, 2025 · 9 comments · May be fixed by #107

Comments

@Nikotos
Copy link

Nikotos commented Jan 7, 2025

I'm facing a weird issue with fetching veAERO NFTs data on Base

Minimal failing example:

with open("veSugar.abi.json", "r") as f:
    VE_SUGAR_ABI = json.loads(f.read())

VE_SUGAR = w3.eth.contract("0x4c5d3925fe65DFeB5A079485136e4De09cb664A5", abi = VE_SUGAR_ABI)

res = VE_SUGAR.functions.all(100, 9_500).call()

which fails with error:

ContractLogicError: ('execution reverted: Governor: managed nft cannot vote', '0x08c379a000000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000000000000000000021476f7665726e6f723a206d616e61676564206e66742063616e6e6f7420766f746500000000000000000000000000000000000000000000000000000000000000')

just in case, web3==6.20.1

@stas
Copy link
Collaborator

stas commented Jan 7, 2025

Confirmed!

We'll take a look.

@Nikotos
Copy link
Author

Nikotos commented Jan 7, 2025

Thanks!

Adding a bit more details:

I've tried for both Abi taken from the basescan

And for Abi generated locally

Same result in both cases

@stas
Copy link
Collaborator

stas commented Jan 7, 2025

Yeah, it won't work since in those 100 venfts limit you're requesting, there's an (m)veNFT -- a special veNFT that is used for Relay and we need to skip fetching the voting power for these (it's always 0 for these btw).

@Nikotos
Copy link
Author

Nikotos commented Jan 7, 2025

got it, thanks

Are there any other way to conveniently load all ~67k NTFs data via batches?

@Nikotos
Copy link
Author

Nikotos commented Jan 7, 2025

btw, it seems that I have created the Solidity implementation for veSugar.all() that doesn't have the formerly mentioned issue. This impfemetatiom allows to obtain all ~67k veNFT data through only 67 .call()'s

However, it does not include the governanceAmount field fetching yet, that is still WIP.

I could share Solidity contract within this repository

Or I could create a new one.

what flow will be the best for us?

@stas
Copy link
Collaborator

stas commented Jan 7, 2025

Thank @Nikotos

Glad it worked for you. We're not interested in Solidity contributions though. If you have time to send a PR for the VeSugar changes, that's great, otherwise we'll patch it sometime tomorrow/next days.

@Nikotos
Copy link
Author

Nikotos commented Jan 7, 2025

I'm just not the fan of the Vyper though.

However, if I will locate the bug veSugar.all() I could try to llm-estimate it's placement in Vyper code

@Nikotos
Copy link
Author

Nikotos commented Jan 7, 2025

Finally located and tested an issue

So! the problem is, veSugar.vy lines 160-161, more precisely the code below:

 if self.gov.address != empty(address):
    governance_amount = staticcall self.gov.getVotes(_id, block.timestamp)

Something like that should do the job:

// Get governance amount if not managed
uint256 governanceAmount = 0;
 if (ve.escrowType(_tokenId) != IVotingEscrow.EscrowType.MANAGED) {
      governanceAmount = gov.getVotes(_tokenId, block.timestamp);
 }

Basically, adding an additional check through ve.escrowType(_tokenId)

@stas stas linked a pull request Jan 8, 2025 that will close this issue
@stas
Copy link
Collaborator

stas commented Jan 8, 2025

There's a fix for this in #107

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

2 participants