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

Account history fails #36

Open
gdbaldw opened this issue Jul 26, 2019 · 13 comments
Open

Account history fails #36

gdbaldw opened this issue Jul 26, 2019 · 13 comments
Assignees
Labels
bug Something isn't working criticial

Comments

@gdbaldw
Copy link
Collaborator

gdbaldw commented Jul 26, 2019

I tested with earlier, sqlite, code. Fails there too. Fails on any account except Faucet account.

Console log:
[2019-07-26 15:31:14,197] ERROR in app: Exception on /account/3b30061de13b8bab1467538bb0edbcee8d7a55ec3f122fd94fb9be5030bf7ddd [GET]
Traceback (most recent call last):
File ".../LibraBrowser/rpc_client.py", line 85, in get_acct_info
len(state.account_state_with_proof.blob.blob) - struct.calcsize('32sQQQQ'))
struct.error: unpack_from requires a buffer of at least 64 bytes

The error is due to missing .blob, which indicates that the account does not exist:
https://github.com/libra/libra/blob/master/types/src/account_state_blob/mod.rs#L97

As demonstrated in the attached Jupyter Notebook: a presumed valid account number from the most recent block update transaction is apparently invalid, because the account_state result does not include blob.
Account_history_fail.zip

Could the error be due to this libra update?
diem/diem@009bc2d

@gdbaldw gdbaldw added the bug Something isn't working label Jul 26, 2019
@gdbaldw
Copy link
Collaborator Author

gdbaldw commented Jul 27, 2019

Here online to discuss.

@Disk1n
Copy link
Owner

Disk1n commented Jul 27, 2019

actually it seems that there are multiple issues.

Issue 1

It seems that the account/address format was changed(?!). I'm not sure but for some reason the client behaves (as you mentioned) as if there is no such account even when we know there is.
I verified that old versions of the official client also fail to retrieve information and in addition when I'm trying to build the latest client it fails. seems like the official distro is broken

Issue 2

This commit broke the formatting of the blob: diem/diem@850513f
becuase we're basically cloning the "CanoncialDesrializer" from here: https://github.com/libra/libra/blob/master/types/src/account_config.rs#L152

@Disk1n Disk1n pinned this issue Jul 27, 2019
@Disk1n
Copy link
Owner

Disk1n commented Jul 27, 2019

For now I'm pushing a pull request for a temporary patch that basically ignores the missing blob. Need to make sure if this bug is on our side or Libra project before we continue. happy if you have any ideas

@Disk1n
Copy link
Owner

Disk1n commented Jul 27, 2019

here is the behavior of the patch I added:
image

@gdbaldw
Copy link
Collaborator Author

gdbaldw commented Jul 27, 2019

I tried also regenerating proto, but did not resolve the issue. Yes, my thought was to bypass the query for now, and I too tentatively concluded testnet is broken.

@gdbaldw
Copy link
Collaborator Author

gdbaldw commented Jul 27, 2019

Looks good

@Disk1n
Copy link
Owner

Disk1n commented Jul 27, 2019

ok, let's keep watching / post a question to the forum. It seems PyLibra also has the same behavior as us: https://github.com/bandprotocol/pylibra/blob/af85d6b22dd7a66396e3bae8a4a30e0aea85ed50/pylibra/client.py#L40

@Disk1n
Copy link
Owner

Disk1n commented Jul 27, 2019

posted a question to the LibraCore section in the official dev forums. waiting moderator approval

@Disk1n
Copy link
Owner

Disk1n commented Jul 28, 2019

@gdbaldw
Copy link
Collaborator Author

gdbaldw commented Jul 29, 2019

Hey, I built a CLI client, funded an account, saw the mint in our browser as a recent transaction, and when I queried the account it was found valid in our browser. So, I looked through other mints and found that several accounts with smaller mint amounts are also valid. My current working theory is that someone is stress testing accounts by minting lots of Libra, and those accounts end up in an invalid state. Anyway, for the current testnet, the following accounts are valid in our browser:

https://librabrowser.io/account/f6876c9a32f0d97f5e27ce389609a66d6518f8ee64f5ade56d4c087deb463af4

https://librabrowser.io/account/2698cb6b5d3891af1d741aa06e9df2e0321a9d18b2aa42a4ac25b2c2a1a0406b

https://librabrowser.io/account/8a7a6260bc943a32a3c7c277d3feb842589eddee838e643d96c5606976bb1301

BTW, procedure for creating CLI client has changed - requires a branch checkout:

git clone https://github.com/libra/libra.git && cd libra
git checkout testnet
./scripts/dev_setup.sh
./scripts/cli/start_cli_testnet.sh

@gdbaldw
Copy link
Collaborator Author

gdbaldw commented Jul 30, 2019

So, from the comments to the thread you initiated, the accounts do not exist, were never created, because all mints exceeded the temporarily hard-coded limit of 1000 Libra.

https://github.com/libra/libra/blob/0ba0013fd8db3bafd23b80e11f15027b5588e6c1/language/stdlib/modules/libra_coin.mvir#L42

We should probably have a better way of handling accounts which do not exist. May want to declare that the account does not exist, but also list any transactions recorded in the blockchain. (Note that the CLI Client misleadingly says the account has a Balance = 0 Libra, does not distinguish between non-existent accounts and accounts having zero balance.)

As a related thought, our total of amount minted should say attempted mint (?), because not all mints are successful.

@Disk1n
Copy link
Owner

Disk1n commented Jul 31, 2019

awesome - I get it now.

yes - we should add behavior for reporting non-existent accounts. even the basic error page is fine, we can kill the patch I made to show "blob missing"

in addition, I agree we should think about total minted again. Or more generally about adding transactions that were rejected into the transaction table. I would like to ignore invalid mints (transactions) but don't want to do it based on hardcoded limit (what if they change it again?) - I'd rather see if we can find the if it was a rejected transaction from the block output. We should research it a bit

@changwu-tw
Copy link
Collaborator

I fixed the problem of non-existent account. #40

  1. Fixed the decode error of account_state.
  2. Fixed acct_details to show data correctly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working criticial
Projects
None yet
Development

No branches or pull requests

3 participants