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

Update XDR definitions for Auth-next #4756

Merged
merged 11 commits into from
Feb 2, 2023

Conversation

2opremio
Copy link
Contributor

@2opremio 2opremio commented Feb 1, 2023

No description provided.

@2opremio
Copy link
Contributor Author

2opremio commented Feb 1, 2023

This failing because the SAC test broke (due to SCO_ACCOUNT_ID being removed).

I need to understand how the token contract changed due to auth-next and update the tests accordingly.

@2opremio 2opremio force-pushed the udpate-xdr-auth-next branch from 32f05e3 to 7786343 Compare February 1, 2023 21:28
@2opremio
Copy link
Contributor Author

2opremio commented Feb 1, 2023

This work is blocked on stellar/stellar-cli#385 , since we need the new token contract code to update the SAC test and make it work.

@2opremio
Copy link
Contributor Author

2opremio commented Feb 1, 2023

We will also need a new Core version supporting the new token contract and that may take a while.

Another option is to merge this with the SAC test broken ...

@tsachiherman thoughts?

@tsachiherman
Copy link
Contributor

Not sure why this PR is blocked on stellar/stellar-cli#385, since compiling the code doesn't really require anything from soroban-tools; but the #385 above is already approved, so it would get merged shortly.

for the core, we should try to use 19.7.1-1178.e352f0012.focal~soroban. If you can't make this work right away, I'd opt into disabling the unit test for now - just so it won't be a blocker. Than, we can keep working on fixing this in parallel. ( please ping me in that case, and I'll open an issue to track that work ).

@2opremio
Copy link
Contributor Author

2opremio commented Feb 2, 2023

since compiling the code doesn't really require anything from soroban-tools

Not the Go code, but it affects the SAC test contract fixture which needs to be redone and Core needs to accept it.

for the core, we should try to use 19.7.1-1178.e352f0012.focal~soroban.

Does that support auth next? (i.e. stellar/stellar-cli#385 ) I doubt it since it was just merged.

I'd opt into disabling the unit test for now - just so it won't be a blocker. Than, we can keep working on fixing this in parallel.

Alright, I think I will need to do this. I will confirm if I do.

Copy link
Contributor

@tsachiherman tsachiherman left a comment

Choose a reason for hiding this comment

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

just one tiny comment.

xdr/scval.go Outdated Show resolved Hide resolved
@2opremio
Copy link
Contributor Author

2opremio commented Feb 2, 2023

@tamirms the randxdr seems to be getting stuck and timing out, any idea of what could be wrong?

@2opremio
Copy link
Contributor Author

2opremio commented Feb 2, 2023

Uhm the TestReingestDB() integration test is also failing.

In particular, it's failing when submitting an invokeHostFunction transaction with: stellar-core exception: unmarshaling did not consume whole message

ERRO[2023-02-02T18:30:20.927+01:00] stellar-core exception: unmarshaling did not consume whole message  pid=95184 req=studioso.local/Qb9eOpM49k-000068 stack="[main.go:30 submitter.go:55 system.go:255 system.go:164 submit_transaction.go:158 handler.go:38 mux.go:425 server.go:2109 cors.go:190 server.go:2109 compress.go:190 server.go:2109 middleware.go:228 server.go:2109 middleware.go:111 server.go:2109 middleware.go:84 server.go:2109 xff_middleware.go:51 server.go:2109 middleware.go:48 server.go:2109 request_id.go:76 server.go:2109 middleware.go:38 server.go:2109 strip.go:25 server.go:2109 mux.go:82 server.go:2947 server.go:1991 asm_arm64.s:1172]"

I need to investigate ...

@2opremio 2opremio force-pushed the udpate-xdr-auth-next branch from 4545bbc to 145650b Compare February 2, 2023 18:16
@2opremio
Copy link
Contributor Author

2opremio commented Feb 2, 2023

This work is blocked on stellar/soroban-tools#385 , since we need the new token contract code to update the SAC test and make it work.

I meant it's blocked on stellar/rs-soroban-env#645 , sorry for the mixup

@tsachiherman
Copy link
Contributor

This work is blocked on stellar/soroban-tools#385 , since we need the new token contract code to update the SAC test and make it work.

I meant it's blocked on stellar/rs-soroban-env#645 , sorry for the mixup

right.. I created #4757 to track the rest of the needed changes. I know that we can't complete the go monorepo changes right now; and that's fine.

@sreuland
Copy link
Contributor

sreuland commented Feb 2, 2023

@2opremio , do you want to add t.Skip("will fix test in #4757") on the SAC tests, so not blocked here. I'm starting on #4757. Also, have seen some transient TestReingestDB fails on gha builds, usually re-run of the failed step, clears.

@2opremio
Copy link
Contributor Author

2opremio commented Feb 2, 2023

@tsachiherman @sreuland This should be good to go once everything is green

Copy link
Contributor

@sreuland sreuland left a comment

Choose a reason for hiding this comment

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

looks good, thanks!

@2opremio 2opremio merged commit 8c3f4c0 into stellar:soroban-xdr-next Feb 2, 2023
@2opremio 2opremio deleted the udpate-xdr-auth-next branch February 2, 2023 23:24
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.

4 participants