Skip to content

Commit

Permalink
Merge branch 'dimitris/remove-old-reject-code' into 'master'
Browse files Browse the repository at this point in the history
chore: Remove old reject code field

Removing the old raw number field of reject code now that the previous change that added the enum field is fully rolled out. 

See merge request dfinity-lab/public/ic!18644
  • Loading branch information
dsarlis committed May 16, 2024
2 parents 51a8096 + 8d09e42 commit d4f1d2e
Show file tree
Hide file tree
Showing 13 changed files with 39 additions and 119 deletions.
62 changes: 17 additions & 45 deletions rs/bitcoin/types/internal/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

use candid::CandidType;
use ic_btc_interface::Network;
use ic_error_types::{RejectCode, TryFromError};
use ic_error_types::RejectCode;
use ic_protobuf::{
bitcoin::v1,
proxy::{try_from_option_field, ProxyDecodeError},
Expand Down Expand Up @@ -436,7 +436,6 @@ impl BitcoinReject {
impl From<&BitcoinReject> for v1::GetSuccessorsReject {
fn from(reject: &BitcoinReject) -> Self {
v1::GetSuccessorsReject {
reject_code_old: reject.reject_code as u64,
message: reject.message.clone(),
reject_code: pbRejectCode::from(reject.reject_code).into(),
}
Expand All @@ -446,28 +445,15 @@ impl From<&BitcoinReject> for v1::GetSuccessorsReject {
impl TryFrom<v1::GetSuccessorsReject> for BitcoinReject {
type Error = ProxyDecodeError;
fn try_from(reject: v1::GetSuccessorsReject) -> Result<Self, Self::Error> {
// A value of 0 for `reject_code_old` indicates that the field
// was not set, i.e. we are past a replica version that has
// populated the new field `reject_code` and we can use that
// instead. Otherwise, we should still use the old field
// when decoding.
let reject_code = if reject.reject_code_old == 0 {
RejectCode::try_from(pbRejectCode::try_from(reject.reject_code).map_err(|_| {
ProxyDecodeError::ValueOutOfRange {
typ: "GetSuccessorsReject::reject_code",
err: format!("value out of range: {}", reject.reject_code),
}
})?)?
} else {
RejectCode::try_from(reject.reject_code_old).map_err(|err| match err {
TryFromError::ValueOutOfRange(range) => ProxyDecodeError::ValueOutOfRange {
typ: "GetSuccessorsReject::reject_code",
err: format!("value out of range: {}", range),
},
})?
};
Ok(BitcoinReject {
reject_code,
reject_code: RejectCode::try_from(
pbRejectCode::try_from(reject.reject_code).map_err(|_| {
ProxyDecodeError::ValueOutOfRange {
typ: "GetSuccessorsReject::reject_code",
err: format!("value out of range: {}", reject.reject_code),
}
})?,
)?,
message: reject.message,
})
}
Expand All @@ -476,7 +462,6 @@ impl TryFrom<v1::GetSuccessorsReject> for BitcoinReject {
impl From<&BitcoinReject> for v1::SendTransactionReject {
fn from(reject: &BitcoinReject) -> Self {
v1::SendTransactionReject {
reject_code_old: reject.reject_code as u64,
message: reject.message.clone(),
reject_code: pbRejectCode::from(reject.reject_code).into(),
}
Expand All @@ -486,28 +471,15 @@ impl From<&BitcoinReject> for v1::SendTransactionReject {
impl TryFrom<v1::SendTransactionReject> for BitcoinReject {
type Error = ProxyDecodeError;
fn try_from(reject: v1::SendTransactionReject) -> Result<Self, Self::Error> {
// A value of 0 for `reject_code_old` indicates that the field
// was not set, i.e. we are past a replica version that has
// populated the new field `reject_code` and we can use that
// instead. Otherwise, we should still use the old field
// when decoding.
let reject_code = if reject.reject_code_old == 0 {
RejectCode::try_from(pbRejectCode::try_from(reject.reject_code).map_err(|_| {
ProxyDecodeError::ValueOutOfRange {
typ: "SendTransactionReject::reject_code",
err: format!("value out of range: {}", reject.reject_code),
}
})?)?
} else {
RejectCode::try_from(reject.reject_code_old).map_err(|err| match err {
TryFromError::ValueOutOfRange(range) => ProxyDecodeError::ValueOutOfRange {
typ: "SendTransactionReject::reject_code",
err: format!("value out of range: {}", range),
},
})?
};
Ok(BitcoinReject {
reject_code,
reject_code: RejectCode::try_from(
pbRejectCode::try_from(reject.reject_code).map_err(|_| {
ProxyDecodeError::ValueOutOfRange {
typ: "SendTransactionReject::reject_code",
err: format!("value out of range: {}", reject.reject_code),
}
})?,
)?,
message: reject.message,
})
}
Expand Down
6 changes: 4 additions & 2 deletions rs/protobuf/def/bitcoin/v1/bitcoin.proto
Original file line number Diff line number Diff line change
Expand Up @@ -113,14 +113,16 @@ message GetSuccessorsResponseComplete {

// A `GetSucceessors` reject response containing additional information about the rejection.
message GetSuccessorsReject {
uint64 reject_code_old = 1;
reserved 1;
reserved "reject_code_old";
types.v1.RejectCode reject_code = 3;
string message = 2;
}

// A `SendTransaction` reject response containing additional information about the rejection.
message SendTransactionReject {
uint64 reject_code_old = 1;
reserved 1;
reserved "reject_code_old";
types.v1.RejectCode reject_code = 3;
string message = 2;
}
3 changes: 2 additions & 1 deletion rs/protobuf/def/state/queues/v1/queues.proto
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,8 @@ message Request {
}

message RejectContext {
uint64 reject_code_old = 1;
reserved 1;
reserved "reject_code_old";
types.v1.RejectCode reject_code = 3;
string reject_message = 2;
}
Expand Down
3 changes: 2 additions & 1 deletion rs/protobuf/def/types/v1/canister_http.proto
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,8 @@ message CanisterHttpResponseContent {
}

message CanisterHttpReject {
uint32 reject_code_old = 1;
reserved 1;
reserved "reject_code_old";
types.v1.RejectCode reject_code = 3;
string message = 2;
}
Expand Down
4 changes: 0 additions & 4 deletions rs/protobuf/src/gen/bitcoin/bitcoin.v1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -180,8 +180,6 @@ pub struct GetSuccessorsResponseComplete {
#[allow(clippy::derive_partial_eq_without_eq)]
#[derive(Clone, PartialEq, ::prost::Message)]
pub struct GetSuccessorsReject {
#[prost(uint64, tag = "1")]
pub reject_code_old: u64,
#[prost(enumeration = "super::super::types::v1::RejectCode", tag = "3")]
pub reject_code: i32,
#[prost(string, tag = "2")]
Expand All @@ -192,8 +190,6 @@ pub struct GetSuccessorsReject {
#[allow(clippy::derive_partial_eq_without_eq)]
#[derive(Clone, PartialEq, ::prost::Message)]
pub struct SendTransactionReject {
#[prost(uint64, tag = "1")]
pub reject_code_old: u64,
#[prost(enumeration = "super::super::types::v1::RejectCode", tag = "3")]
pub reject_code: i32,
#[prost(string, tag = "2")]
Expand Down
4 changes: 0 additions & 4 deletions rs/protobuf/src/gen/registry/bitcoin.v1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -163,8 +163,6 @@ pub struct GetSuccessorsResponseComplete {
#[allow(clippy::derive_partial_eq_without_eq)]
#[derive(Clone, PartialEq, ::prost::Message)]
pub struct GetSuccessorsReject {
#[prost(uint64, tag = "1")]
pub reject_code_old: u64,
#[prost(enumeration = "super::super::types::v1::RejectCode", tag = "3")]
pub reject_code: i32,
#[prost(string, tag = "2")]
Expand All @@ -174,8 +172,6 @@ pub struct GetSuccessorsReject {
#[allow(clippy::derive_partial_eq_without_eq)]
#[derive(Clone, PartialEq, ::prost::Message)]
pub struct SendTransactionReject {
#[prost(uint64, tag = "1")]
pub reject_code_old: u64,
#[prost(enumeration = "super::super::types::v1::RejectCode", tag = "3")]
pub reject_code: i32,
#[prost(string, tag = "2")]
Expand Down
4 changes: 0 additions & 4 deletions rs/protobuf/src/gen/state/bitcoin.v1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -163,8 +163,6 @@ pub struct GetSuccessorsResponseComplete {
#[allow(clippy::derive_partial_eq_without_eq)]
#[derive(Clone, PartialEq, ::prost::Message)]
pub struct GetSuccessorsReject {
#[prost(uint64, tag = "1")]
pub reject_code_old: u64,
#[prost(enumeration = "super::super::types::v1::RejectCode", tag = "3")]
pub reject_code: i32,
#[prost(string, tag = "2")]
Expand All @@ -174,8 +172,6 @@ pub struct GetSuccessorsReject {
#[allow(clippy::derive_partial_eq_without_eq)]
#[derive(Clone, PartialEq, ::prost::Message)]
pub struct SendTransactionReject {
#[prost(uint64, tag = "1")]
pub reject_code_old: u64,
#[prost(enumeration = "super::super::types::v1::RejectCode", tag = "3")]
pub reject_code: i32,
#[prost(string, tag = "2")]
Expand Down
2 changes: 0 additions & 2 deletions rs/protobuf/src/gen/state/state.queues.v1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,8 +86,6 @@ pub struct Request {
#[allow(clippy::derive_partial_eq_without_eq)]
#[derive(Clone, PartialEq, ::prost::Message)]
pub struct RejectContext {
#[prost(uint64, tag = "1")]
pub reject_code_old: u64,
#[prost(enumeration = "super::super::super::types::v1::RejectCode", tag = "3")]
pub reject_code: i32,
#[prost(string, tag = "2")]
Expand Down
4 changes: 0 additions & 4 deletions rs/protobuf/src/gen/types/bitcoin.v1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -180,8 +180,6 @@ pub struct GetSuccessorsResponseComplete {
#[allow(clippy::derive_partial_eq_without_eq)]
#[derive(Clone, PartialEq, ::prost::Message)]
pub struct GetSuccessorsReject {
#[prost(uint64, tag = "1")]
pub reject_code_old: u64,
#[prost(enumeration = "super::super::types::v1::RejectCode", tag = "3")]
pub reject_code: i32,
#[prost(string, tag = "2")]
Expand All @@ -192,8 +190,6 @@ pub struct GetSuccessorsReject {
#[allow(clippy::derive_partial_eq_without_eq)]
#[derive(Clone, PartialEq, ::prost::Message)]
pub struct SendTransactionReject {
#[prost(uint64, tag = "1")]
pub reject_code_old: u64,
#[prost(enumeration = "super::super::types::v1::RejectCode", tag = "3")]
pub reject_code: i32,
#[prost(string, tag = "2")]
Expand Down
2 changes: 0 additions & 2 deletions rs/protobuf/src/gen/types/state.queues.v1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,6 @@ pub struct Request {
#[allow(clippy::derive_partial_eq_without_eq)]
#[derive(Clone, PartialEq, ::prost::Message)]
pub struct RejectContext {
#[prost(uint64, tag = "1")]
pub reject_code_old: u64,
#[prost(enumeration = "super::super::super::types::v1::RejectCode", tag = "3")]
pub reject_code: i32,
#[prost(string, tag = "2")]
Expand Down
2 changes: 0 additions & 2 deletions rs/protobuf/src/gen/types/types.v1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1478,8 +1478,6 @@ pub mod canister_http_response_content {
#[allow(clippy::derive_partial_eq_without_eq)]
#[derive(Clone, PartialEq, ::prost::Message)]
pub struct CanisterHttpReject {
#[prost(uint32, tag = "1")]
pub reject_code_old: u32,
#[prost(enumeration = "RejectCode", tag = "3")]
pub reject_code: i32,
#[prost(string, tag = "2")]
Expand Down
36 changes: 9 additions & 27 deletions rs/types/types/src/batch/canister_http.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use crate::{
CountBytes, Time,
};
use ic_base_types::{NodeId, PrincipalId, RegistryVersion};
use ic_error_types::{RejectCode, TryFromError};
use ic_error_types::RejectCode;
#[cfg(test)]
use ic_exhaustive_derive::ExhaustiveSet;
use ic_protobuf::{
Expand Down Expand Up @@ -164,7 +164,6 @@ impl From<&CanisterHttpResponseContent> for pb::CanisterHttpResponseContent {
}
CanisterHttpResponseContent::Reject(error) => {
pb::canister_http_response_content::Status::Reject(pb::CanisterHttpReject {
reject_code_old: error.reject_code as u32,
message: error.message.clone(),
reject_code: pb::RejectCode::from(error.reject_code).into(),
})
Expand All @@ -190,32 +189,15 @@ impl TryFrom<pb::CanisterHttpResponseContent> for CanisterHttpResponseContent {
CanisterHttpResponseContent::Success(payload)
}
pb::canister_http_response_content::Status::Reject(error) => {
// A value of 0 for `reject_code_old` indicates that the field
// was not set, i.e. we are past a replica version that has
// populated the new field `reject_code` and we can use that
// instead. Otherwise, we should still use the old field
// when decoding.
let reject_code = if error.reject_code_old == 0 {
RejectCode::try_from(pb::RejectCode::try_from(error.reject_code).map_err(
|_| ProxyDecodeError::ValueOutOfRange {
typ: "reject_code",
err: format!("value out of range: {}", error.reject_code),
},
)?)?
} else {
RejectCode::try_from(error.reject_code_old as u64).map_err(
|err| match err {
TryFromError::ValueOutOfRange(range) => {
ProxyDecodeError::ValueOutOfRange {
typ: "reject_code",
err: format!("value out of range: {}", range),
}
}
},
)?
};
CanisterHttpResponseContent::Reject(CanisterHttpReject {
reject_code,
reject_code: RejectCode::try_from(
pb::RejectCode::try_from(error.reject_code).map_err(|_| {
ProxyDecodeError::ValueOutOfRange {
typ: "reject_code",
err: format!("value out of range: {}", error.reject_code),
}
})?,
)?,
message: error.message,
})
}
Expand Down
26 changes: 5 additions & 21 deletions rs/types/types/src/messages/inter_canister.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::{
ingress::WasmResult, time::CoarseTime, CanisterId, CountBytes, Cycles, Funds, NumBytes, Time,
};
use ic_error_types::{RejectCode, TryFromError, UserError};
use ic_error_types::{RejectCode, UserError};
#[cfg(test)]
use ic_exhaustive_derive::ExhaustiveSet;
use ic_management_canister_types::{
Expand Down Expand Up @@ -609,7 +609,6 @@ impl TryFrom<pb_queues::Request> for Request {
impl From<&RejectContext> for pb_queues::RejectContext {
fn from(rc: &RejectContext) -> Self {
Self {
reject_code_old: rc.code as u64,
reject_message: rc.message.clone(),
reject_code: pb_types::RejectCode::from(rc.code).into(),
}
Expand All @@ -620,28 +619,13 @@ impl TryFrom<pb_queues::RejectContext> for RejectContext {
type Error = ProxyDecodeError;

fn try_from(rc: pb_queues::RejectContext) -> Result<Self, Self::Error> {
// A value of 0 for `reject_code_old` indicates that the field
// was not set, i.e. we are past a replica version that has
// populated the new field `reject_code` and we can use that
// instead. Otherwise, we should still use the old field
// when decoding.
let code = if rc.reject_code_old == 0 {
RejectCode::try_from(pb_types::RejectCode::try_from(rc.reject_code).map_err(|_| {
ProxyDecodeError::ValueOutOfRange {
Ok(RejectContext {
code: RejectCode::try_from(pb_types::RejectCode::try_from(rc.reject_code).map_err(
|_| ProxyDecodeError::ValueOutOfRange {
typ: "RejectContext",
err: format!("Unexpected value for reject code {}", rc.reject_code),
}
})?)?
} else {
rc.reject_code_old.try_into().map_err(|err| match err {
TryFromError::ValueOutOfRange(code) => ProxyDecodeError::ValueOutOfRange {
typ: "RejectContext",
err: code.to_string(),
},
})?
};
Ok(RejectContext {
code,
)?)?,
message: rc.reject_message,
})
}
Expand Down

0 comments on commit d4f1d2e

Please sign in to comment.