Skip to content

Facilitator get signers impl#4

Merged
sasurobert merged 9 commits intomainfrom
facilitator-getSigners-impl
Jan 27, 2026
Merged

Facilitator get signers impl#4
sasurobert merged 9 commits intomainfrom
facilitator-getSigners-impl

Conversation

@sasurobert
Copy link
Owner

Description

Tests

Checklist

  • I have formatted and linted my code
  • All new and existing tests pass
  • My commits are signed (required for merge) -- you may need to rebase if you initially pushed unsigned commits

gasLimit = uint64(gl)
}
version := uint32(1)
version := uint32(2)

Choose a reason for hiding this comment

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

here we are hardcoding to relayed. lower, in the facilitator we have a conditional if s.signer != nil and we choose between signing a relayed transaction and simple. also, in our docs we have this: assetTransferMethod: "direct/relayed". I think this should be the source of truth. So on this case, we should use that field and set the correct version

Copy link
Owner Author

Choose a reason for hiding this comment

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

done

Choose a reason for hiding this comment

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

Can we please assume a type and use that type only and error if we cannot cast to it? It looks ugly to try and cast every size possible

Copy link
Owner Author

Choose a reason for hiding this comment

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

done

}
}
}
relayer, _ := requirements.Extra["relayer"].(string)

Choose a reason for hiding this comment

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

we need to fix this, it casts it two times. once, and error if not ok

Copy link
Owner Author

Choose a reason for hiding this comment

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

done

Choose a reason for hiding this comment

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

can we cast directly to []string and remove the if argStr, ok := arg.(string); ok {?

Copy link
Owner Author

Choose a reason for hiding this comment

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

done

Choose a reason for hiding this comment

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

I think this is not correct as it can be a SC call. And the info in how much that SC call would cost would be in our Extra field which is handled above. We can handle there also the native vs esdt

Copy link
Owner Author

Choose a reason for hiding this comment

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

I do not think it is in the extra field, it is in the gasLimit field.

Copy link
Owner Author

Choose a reason for hiding this comment

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

so I removed gasLimit completely.

Choose a reason for hiding this comment

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

"Empty slice declaration using a literal " warning (optional to fix)

Copy link
Owner Author

Choose a reason for hiding this comment

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

done

Choose a reason for hiding this comment

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

Optional: If not too much overhead (since there are a lot of fields) it would be nice if we can refactor this if/else to some helper functions, like:

if if asset == multiversx.NativeTokenTicker {
  receiver, value, gaslLimit, data := s.getArgumentsForNativeTransfert(...)
} else {
  .... := s.getArgumentsForESDT(...)
}

I think it would be easier to read through

Copy link
Owner Author

Choose a reason for hiding this comment

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

done


hash, err := s.proxy.SendTransaction(ctx, &tx)
var hash string
if s.signer != nil {

Choose a reason for hiding this comment

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

We should use transfer type here and return error if we don't have a signer (to be consistent)

Copy link
Owner Author

Choose a reason for hiding this comment

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

done

}

// Ensure empty string if still missing (simulation requirement if not signed)
if tx.RelayerSignature == "" {

Choose a reason for hiding this comment

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

please remove this :))

Copy link
Owner Author

Choose a reason for hiding this comment

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

done

@sasurobert sasurobert merged commit 6b5ac60 into main Jan 27, 2026
9 of 14 checks passed
@sasurobert sasurobert deleted the facilitator-getSigners-impl branch January 27, 2026 19:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants