Skip to content

Synchronous container operations#358

Merged
cthulhu-rider merged 1 commit intomasterfrom
container-async-await
Dec 2, 2025
Merged

Synchronous container operations#358
cthulhu-rider merged 1 commit intomasterfrom
container-async-await

Conversation

@cthulhu-rider
Copy link
Copy Markdown
Contributor

No description provided.

@cthulhu-rider cthulhu-rider marked this pull request as ready for review December 1, 2025 11:14
roman-khimov
roman-khimov previously approved these changes Dec 1, 2025
@roman-khimov
Copy link
Copy Markdown
Member

Conflicts

Closes #353.

Signed-off-by: Leonard Lyubich <leonard@morphbits.io>
Comment thread container/service.proto
// verified by Inner Ring nodes. After one more block in FS chain, the container
// is added into smart contract storage.
// Sends transaction calling contract method to create container, and waits
// for the transaction to be executed. Deadline is determined by the
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

executed -> accepted?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

why u think this is better?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

i think this better explains our blockchain nature of transactions. they are executed in test mode too, but may never be accepted

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

"accepted" may also be understood as old behavior with request acceptance

Comment thread status/types.proto
CONTAINER_LOCKED = 2;

// [**3075**] Async container operation timed out.
CONTAINER_AWAIT_TIMEOUT = 3;
Copy link
Copy Markdown
Member

@carpawell carpawell Dec 1, 2025

Choose a reason for hiding this comment

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

i thought that we wanted to add a more general code like HTTP's 504 Gateway Timeout or smth like that when we (SN) are not responsible for execution and only wait for other apps to process smth. SetExtendedACL with CONTAINER_AWAIT_TIMEOUT response confuses me

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

this is not meaningful to me for now. In theory, 504 can relate smth other than the chain within the same op. Then details would be different. For example, tx ID against some other resource detail

SetExtendedACL with CONTAINER_AWAIT_TIMEOUT response confuses me

SetEACL is an op on the container. It could be EACL_AWAIT_TIMEOUT, but it would be a pure alias anyway

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

i meant that in general we have some things that should be paid by Proxy, multisigned by the alphabet/container nodes, be accepted to FS chain, so many things may go wrong. i think it would be better to have a common chain timeouted/tx was not accepted code and reuse it everywhere when SN is not responsible for the outcome. otherwise, we may end up with multiple codes that mean the same

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

But that's container operations only and CONTAINER_AWAIT_TIMEOUT, isn't it?

Copy link
Copy Markdown
Contributor Author

@cthulhu-rider cthulhu-rider Dec 2, 2025

Choose a reason for hiding this comment

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

i meant that in general we have some things that should be paid by Proxy, multisigned by the alphabet/container nodes, be accepted to FS chain, so many things may go wrong

most of these things are internal problems transmitted with corresponding status

have a common chain timeouted/tx was not accepted

on the one hand, this could happen to any FS chain transaction. On the other hand, current API doesn't offer any other transaction-based ops. I'm concerned about details that might be contract-specific

we already raised similar topic earlier #343 (comment). The decision was made for specific statuses at that moment. I jut followed it here. Moreover, we can switch to a new code at any time if any

Copy link
Copy Markdown
Member

@carpawell carpawell Dec 2, 2025

Choose a reason for hiding this comment

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

But that's container operations only and CONTAINER_AWAIT_TIMEOUT, isn't it?

On the other hand, current API doesn't offer any other transaction-based ops

we have metadata at least. we worked, work, and will work with blockchains, so there may appear more situations when txs are sent by SNs but handled somewhere later in chain.

we already raised similar topic earlier #343 (comment)

imo, a different example. "size" is unknown for other operations, and i am not sure this can be understood in a different context, also, quotas is a certain feature that we released, and such statuses are going to be thrown from time to time. while txs that have not been accepted is a general thing to me (like "tx" term itself)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

i do not insist btw

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

"size" is unknown for other operations

to me, this arg is similar to

current API doesn't offer any other transaction-based ops

@cthulhu-rider cthulhu-rider merged commit ff6de9a into master Dec 2, 2025
3 checks passed
@cthulhu-rider cthulhu-rider deleted the container-async-await branch December 2, 2025 12:39
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.

3 participants