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.

Sign up for GitHub

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

fix(PE-7196): add msg.reply to all apis #72

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from
+30 −24
Open

fix(PE-7196): add msg.reply to all apis #72

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
32da7f8
fix(reply): add msg.reply to all apis
atticusofsparta Jan 29, 2025
162b3ed
fix(version): update version and changelog
atticusofsparta Jan 29, 2025
a7e4c53
fix(reply): dont use utils.Send in boot because it will crash memory
atticusofsparta Jan 29, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,12 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).

### Fixed

- Added msg.reply to all APIs

## [13] - [zh4at_Y_GKJMD3SOkZ5Yx7mG2JRRLc89huEOspPtHq4] - (2025-1-29)

### Fixed

- Removed print with ID in Handler wrapper since ID can be null on dry runs.

## [12] - [mwCMAjglwV_96oEMEIi5epg_QXElOMzEcLkCUeQyGGo] - (2025-1-28)
Expand Down
45 changes: 22 additions & 23 deletions src/common/main.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ function ant.init()
local recipient = msg.Tags.Recipient
if msg.From ~= Owner then
utils.Send(msg, {
Target = msg.From,
Copy link

Choose a reason for hiding this comment

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

Would it make sense to set this in utils.Send automatically if not present rather than making it the responsibility of the caller?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes it would make sense to add it there - technically its not needed for msg.reply, and we only add this because of ao.send

If we don't, we actually send an invalid message if msg.reply is not present.

Im actually wondering if we even need to wrap ao.send like this, because we KNOW what AOS module we are building. This wrapping pattern is only for backwards compatibility with older AOS process, and with our reassign-to-evolve pattern it will be a new module of our choice anyways.

Also we may not always want to "reply". Sometimes we just want to send. Using reply probably doesnt hurt, but IDK what future integrations may be influenced with these extra tags.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

and in fact if my suspicions are correct, in that adding function calls is messing with stack memory, then we wanna be careful about that.

Action = "Transfer-Error",
["Message-Id"] = msg.Id,
Error = "Insufficient Balance!",
Expand All @@ -108,6 +109,7 @@ function ant.init()
local balRes = balances.balance(addressToCheck, msg.Tags["Allow-Unsafe-Addresses"])

utils.Send(msg, {
Target = msg.From,
Action = "Balance-Notice",
Balance = tostring(balRes),
Ticker = Ticker,
Expand All @@ -123,13 +125,14 @@ function ant.init()
bals[k] = tostring(v)
end

utils.Send(msg, { Data = json.encode(bals) })
utils.Send(msg, { Target = msg.From, Data = json.encode(bals) })
end)

createActionHandler(TokenSpecActionMap.TotalSupply, function(msg)
assert(msg.From ~= ao.id, "Cannot call Total-Supply from the same process!")

utils.Send(msg, {
Target = msg.From,
Action = "Total-Supply",
Data = tostring(TotalSupply),
Ticker = Ticker,
Expand All @@ -149,6 +152,7 @@ function ant.init()
Handlers = utils.getHandlerNames(Handlers),
}
utils.Send(msg, {
Target = msg.From,
Action = "Info-Notice",
Tags = info,
Data = json.encode(info),
Expand Down Expand Up @@ -225,8 +229,8 @@ function ant.init()
return initialize.initializeANTState(msg.Data)
end)

createActionHandler(ActionMap.State, function(msg)
notices.notifyState(msg, msg.From)
createActionHandler(ActionMap.State, function()
return utils.getState()
end)

-- IO Network Contract Handlers
Expand All @@ -248,7 +252,7 @@ function ant.init()
Name = name,
})

ao.send({
utils.Send(msg, {
Target = msg.From,
Action = "Release-Name-Notice",
Initiator = msg.From,
Expand All @@ -275,7 +279,7 @@ function ant.init()
["Process-Id"] = antProcessIdToReassign,
})

ao.send({
utils.Send(msg, {
Target = msg.From,
Action = "Reassign-Name-Notice",
Initiator = msg.From,
Expand Down Expand Up @@ -329,6 +333,7 @@ function ant.init()
AOS provides a _boot handler that is designed to load Lua code on boot.
This handler OVERRIDES this and replaces it with our ANT state initialization handler.

NOTE: if we use utils.Send here memory blows up for some reason
]]
Handlers.once("_boot", function(msg)
return msg.Tags.Type == "Process" and Owner == msg.From
Expand All @@ -339,28 +344,22 @@ function ant.init()
initialize.initializeANTState(msg.Data)
end, utils.errorHandler)
if not status then
utils.Send(
msg,
notices.addForwardedTags(msg, {
Target = Owner,
Error = res or "",
Data = res or "",
Action = "Invalid-Boot-Notice",
["Message-Id"] = msg.Id,
})
)
ao.send(notices.addForwardedTags(msg, {
Copy link
Collaborator

Choose a reason for hiding this comment

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

where is ao.send? and does it have the wrapper logic for using msg.reply?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

https://github.com/ar-io/ar-io-ant-process/pull/72/files#diff-2347104afd1b651068d7a89f9caf67ca6a1d9872d578048f8f0c4410373d3c4fR336

To be clear, this memory blowup happens not during compile time or at any other message, but rather on eval specifically, and specifically of our bundled lua.

I did investigate as to why, and it seems that adding utils.Send increases the complexity of the function call, expanding the memory it uses on the stack, causing the stack to give a memory out of bounds error - thats my theory, anyway. I think for the moment we should leave utils.Send out of this call and further investigate in another PR/SPIKE as to what options are available to us here.

The main solution - apart from not including the util - is to simply raise the stack memory. However I don't want to create a lua script that can be eval'd into one module and not another, especially since thats how our current evolve workflow works on the arns portal.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As for where ao.send is, its from AOS - ao is available as a global. Its actually what utils.Send uses internally.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
ao.send(notices.addForwardedTags(msg, {
utils.Send(notices.addForwardedTags(msg, {

Target = Owner,
Error = res or "",
Data = res or "",
Action = "Invalid-Boot-Notice",
["Message-Id"] = msg.Id,
}))
end
end

if Owner then
utils.Send(
msg,
notices.credit({
From = msg.From,
Sender = Owner,
Recipient = Owner,
})
)
ao.send(notices.credit({
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
ao.send(notices.credit({
utils.Send(notices.credit({

From = msg.From,
Sender = Owner,
Recipient = Owner,
}))
end

if AntRegistryId then
Expand Down
1 change: 1 addition & 0 deletions test/evolve.test.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ describe('aos Evolve', async () => {
},
evolveResult.Memory,
);

assert(evalResult.Output.data);
assert(evalResult.Output.data.includes('info'));
});
Expand Down
2 changes: 1 addition & 1 deletion version.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
// TODO: make this an auto generated file

export default '12';
export default '13';
Loading